Repository: pravega/zookeeper-operator
Pull request #184 (2020-05-27 16:16:01)

Issue 183: Prefixing service ports with tcp to comply with istio standards
Fixes https://github.com/pravega/zookeeper-operator/issues/183 based on this istio issue https://github.com/istio/istio/issues/23991

Also added the client port (2181) to the headless service as many systems that use ZK (Kafka, Solr) will talk to ZKs client port through the headless service. 

codecov-commenter: # [Codecov](https://codecov.io/gh/pravega/zookeeper-operator/pull/184?src=pr&el=h1) Report
> Merging [#184](https://codecov.io/gh/pravega/zookeeper-operator/pull/184?src=pr&el=desc) into [master](https://codecov.io/gh/pravega/zookeeper-operator/commit/45f8a92e7db437409893372a5d52542f2e4c2d8d&el=desc) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/pravega/zookeeper-operator/pull/184/graphs/tree.svg?width=650&height=150&src=pr&token=04r424HENM)](https://codecov.io/gh/pravega/zookeeper-operator/pull/184?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #184   +/-   ##
=======================================
  Coverage   53.38%   53.38%           
=======================================
  Files           7        7           
  Lines        1107     1107           
=======================================
  Hits          591      591           
  Misses        468      468           
  Partials       48       48           
```


| [Impacted Files](https://codecov.io/gh/pravega/zookeeper-operator/pull/184?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/zk/generators.go](https://codecov.io/gh/pravega/zookeeper-operator/pull/184/diff?src=pr&el=tree#diff-cGtnL3prL2dlbmVyYXRvcnMuZ28=) | `86.44% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pravega/zookeeper-operator/pull/184?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pravega/zookeeper-operator/pull/184?src=pr&el=footer). Last update [45f8a92...a99914e](https://codecov.io/gh/pravega/zookeeper-operator/pull/184?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

AceHack: Can't wait!! this will really help our istio deployment.
Prabhaker24: @whitleykeith I am testing these changes we will merge this pr once the testing is completed.
Prabhaker24: @whitleykeith can you please rebase to master as well
whitleykeith: rebased
anishakj: After doing rebase,  changes are  not showing correctly. You may have to just pull from master, rather than doing rebase
whitleykeith: @anishakj Did you need me to pull from master again? I noticed the email but I don't see the comment you made here
anishakj: > @anishakj Did you need me to pull from master again? I noticed the email but I don't see the comment you made here

There are 2 issues, there is a conflict in generators.go as well as the change is not rebased on master.
Please follow https://github.com/pravega/zookeeper-operator/wiki/Contributing#issues-and-pull-request and in case of any issues, let us know
whitleykeith: @anishakj check it now. I ended up messing up some reverts but I think the diff looks good now.

Patch:
diff --git a/pkg/zk/generators.go b/pkg/zk/generators.go
index 1c0ce6a..02b761f 100644
--- a/pkg/zk/generators.go
+++ b/pkg/zk/generators.go
@@ -163,7 +163,7 @@ func makeZkPodSpec(z *v1beta1.ZookeeperCluster) v1.PodSpec {
 func MakeClientService(z *v1beta1.ZookeeperCluster) *v1.Service {
 	ports := z.ZookeeperPorts()
 	svcPorts := []v1.ServicePort{
-		{Name: "client", Port: ports.Client},
+		{Name: "tcp-client", Port: ports.Client},
 	}
 	return makeService(z.GetClientServiceName(), svcPorts, true, z)
 }
@@ -193,9 +193,9 @@ func MakeConfigMap(z *v1beta1.ZookeeperCluster) *v1.ConfigMap {
 func MakeHeadlessService(z *v1beta1.ZookeeperCluster) *v1.Service {
 	ports := z.ZookeeperPorts()
 	svcPorts := []v1.ServicePort{
-		{Name: "quorum", Port: ports.Quorum},
-		{Name: "leader-election", Port: ports.Leader},
-		{Name: "metrics", Port: ports.Metrics},
+		{Name: "tcp-client", Port: ports.Client},
+		{Name: "tcp-quorum", Port: ports.Quorum},
+		{Name: "tcp-leader-election", Port: ports.Leader},
 	}
 	return makeService(headlessSvcName(z), svcPorts, false, z)
 }


Test patch:
diff --git a/pkg/zk/generators_test.go b/pkg/zk/generators_test.go
index f247da5..1c1c2d0 100644
--- a/pkg/zk/generators_test.go
+++ b/pkg/zk/generators_test.go
@@ -138,7 +138,7 @@ var _ = Describe("Generators Spec", func() {
 		})
 
 		It("should have a client port", func() {
-			p, err := utils.ServicePortByName(s.Spec.Ports, "client")
+			p, err := utils.ServicePortByName(s.Spec.Ports, "tcp-client")
 			Ω(err).To(BeNil())
 			Ω(p.Port).To(BeEquivalentTo(2181))
 		})
@@ -177,13 +177,13 @@ var _ = Describe("Generators Spec", func() {
 		})
 
 		It("should have a quorum port", func() {
-			p, err := utils.ServicePortByName(s.Spec.Ports, "quorum")
+			p, err := utils.ServicePortByName(s.Spec.Ports, "tcp-quorum")
 			Ω(err).To(BeNil())
 			Ω(p.Port).To(BeEquivalentTo(2888))
 		})
 
 		It("should have a leader port", func() {
-			p, err := utils.ServicePortByName(s.Spec.Ports, "leader-election")
+			p, err := utils.ServicePortByName(s.Spec.Ports, "tcp-leader-election")
 			Ω(err).To(BeNil())
 			Ω(p.Port).To(BeEquivalentTo(3888))
 		})