Conversation
|
@tuteng:Thanks for your contribution. For this PR, do we need to update docs? |
There was a problem hiding this comment.
Pull request overview
This PR updates the Toolset chart templates so that when Toolset is configured to use the proxy, it can target the proxy “ingress” service (when proxy ingress is enabled) instead of always using the proxy headless service, improving compatibility for pulsar-admin usage from within the Toolset pod.
Changes:
- Add a new Helm helper (
toolset.proxy.service.host) to choose the proxy service hostname (-ingressvs-headless) based on proxy ingress settings. - Update Toolset configmap proxy URLs (
webServiceUrl,brokerServiceUrl) to use the new helper. - Update
toolset.web.service.url(used bypulsarctl.config) to use the new helper.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| charts/sn-platform/templates/toolset/toolset-configmap.yaml | Switches proxy connection URLs to use toolset.proxy.service.host. |
| charts/sn-platform/templates/toolset/_toolset.tpl | Introduces toolset.proxy.service.host and updates toolset service URL helper to use it. |
| charts/sn-platform-slim/templates/toolset/toolset-configmap.yaml | Same change as sn-platform: proxy URLs now use toolset.proxy.service.host. |
| charts/sn-platform-slim/templates/toolset/_toolset.tpl | Same change as sn-platform: adds helper and updates toolset service URL helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
(If this PR fixes a github issue, please add
Fixes #<xyz>.)Fixes #
(or if this PR is one task of a github issue, please add
Master Issue: #<xyz>to link to the master issue.)Master Issue: #
Motivation
Explain here the context, and why you're making that change. What is the problem you're trying to solve.
Modifications
Describe the modifications you've done.
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Documentation
Check the box below.
Need to update docs?
doc-required(If you need help on updating docs, create a doc issue)
no-need-doc(Please explain why)
doc(If this PR contains doc changes)