fix(helm): derive grpcEndpoint from chart context#1241
Open
TaylorMutch wants to merge 2 commits intomainfrom
Open
fix(helm): derive grpcEndpoint from chart context#1241TaylorMutch wants to merge 2 commits intomainfrom
TaylorMutch wants to merge 2 commits intomainfrom
Conversation
The chart hardcoded server.grpcEndpoint to https://openshell.openshell.svc.cluster.local:8080, which only matched the in-cluster Service DNS for the standard release name and namespace. A new helper now builds <scheme>://<fullname>.<namespace>.svc.cluster.local:<port> from chart context, picking the scheme from server.disableTls. An explicit server.grpcEndpoint override is passed through verbatim.
helm-k3s-local.sh derives the cluster name from the current branch suffix. Long branch names produced names exceeding k3d's 32-char cap and failed deep inside k3d cluster create with a confusing validation error. cmd_create now bails out before invoking docker/k3d with a copy-pasteable HELM_K3S_CLUSTER_NAME override hint. Status, start, stop, delete, and help remain unaffected so an over-long derived name does not block diagnostics.
TaylorMutch
requested review from
a team,
derekwaynecarr,
maxamillion and
mrunalp
as code owners
Contributor
|
Tested and lgtm 👍 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
server.grpcEndpointwas hardcoded invalues.yamltohttps://openshell.openshell.svc.cluster.local:8080, which only matched the in-cluster Service DNS for releases namedopenshellin namespaceopenshell. The chart now derives the endpoint from chart context, so the default works for any release name, namespace, or service port without an override. An explicitserver.grpcEndpointis used verbatim.Also bails out early in
helm-k3s-local.shwhen the branch-derived k3d cluster name exceeds k3d's 32-char cap, with a copy-pasteable override hint.Related Issue
n/a
Changes
openshell.grpcEndpointhelper in_helpers.tplbuilds<scheme>://<fullname>.<namespace>.svc.cluster.local:<service.port>from chart context, picking the scheme fromserver.disableTls. An explicitserver.grpcEndpointis passed through verbatim.templates/statefulset.yamlconsumes the helper forOPENSHELL_GRPC_ENDPOINT.values.yamldefaultsserver.grpcEndpointto empty with a comment explaining auto-derivation.deploy/kube/manifests/openshell-helmchart.yamldrops the redundant hardcoded value.examples/gateway-deploy-connect.mddrops the now-unnecessary--set server.grpcEndpoint=...flag.tasks/scripts/helm-k3s-local.shvalidates the cluster name length incmd_createbefore calling docker/k3d.Testing
mise run pre-commitpasseshelm templaterendered correctly across: default mTLS, plaintext (disableTls=true), custom release name + namespace, explicit override (verbatim), explicit override withdisableTls=true(still verbatim)mise run helm:skaffold:runagainst a local k3d cluster — pod healthy, resolved envOPENSHELL_GRPC_ENDPOINT=http://openshell.openshell.svc.cluster.local:8080helm-k3s-local.sh createwith a long branch suffix exits early with the suggestedHELM_K3S_CLUSTER_NAME=...override;status,--help,start,stop,deleteare unaffectedChecklist