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

Fix #152 Add Helm chart support for Istio port naming (attempt 2) #162

Merged
merged 3 commits into from
Sep 10, 2021

Conversation

frankjkelly
Copy link
Contributor

@frankjkelly frankjkelly commented Sep 8, 2021

Fixes #158 (was incorrectly listed as 152)

Motivation

Support prefix in front of port names to abide by Istio protocol rules
https://istio.io/latest/docs/ops/configuration/traffic-management/protocol-selection/#explicit-protocol-selection

Modifications

Support adding a prefix

  • pulsar -> tcp-pulsar
  • pulsarssl --> tls-pulsarssl etc

Verifying this change

  • Make sure that the change passes the CI checks.

NOTE: @lhotari @sijie this is designed to address feedback in #159 - just easier to create a net new PR

@frankjkelly frankjkelly changed the title Fix #152 Helm chart does not support Istio port naming (attempt 2) Fix #152 Add Helm chart support for Istio port naming (attempt 2) Sep 9, 2021
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@codelipenghui codelipenghui merged commit 5b10f48 into apache:master Sep 10, 2021
@josephglanville
Copy link

I think this was meant to close #158 but closed my PR #152 instead.

sijie pushed a commit that referenced this pull request Nov 9, 2021
…t name (#172)

Fixes #158 (This is the second PR - see also #162)

### Motivation

* All non-standard port-names need a proper protocol prefix to support Istio
 https://istio.io/latest/docs/ops/configuration/traffic-management/protocol-selection/#explicit-protocol-selection
 
### Modifications

Add the prefix value before `bookie`
pgier pushed a commit to pgier/pulsar-helm-chart that referenced this pull request Apr 22, 2022
In our current test logs,we're failing to complete the helm deployment because of this error:

```
ready.go:258: [debug] Service does not have load balancer ingress IP address: pulsar-lzg9asf1t7/pulsar-adminconsole
```

We don't need to test with load balancers. Instead, use ClusterIP to ensure that the targets properly come up
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.

Helm chart does not support Istio: Add support for Istio standard port naming
5 participants