Issue 430: Exposing Segmentstore Service on single IP #431

Prabhaker24 · 2020-07-27T13:57:19Z

Change log description

Exposing Segmentstore Service on single IP address and Different ports

Purpose of the change

fixes #430

What the code does

Currently, Segment store pods are exposed using external IP address which is given by Service, Each pod has its own service which has a unique IP address which can be used to communicate to the Segment store, after this change we can expose segment store services on the same I/P address and different port

How to verify it

To verify this change we need Bare metal kubernetes cluster and we need to specify the below fields for it to work in
the manifest

pravega:
    . . .
    segmentStoreLoadBalancerIP: "10.243.39.103"
    segmentStoreExternalTrafficPolicy: "cluster"
    segmentStoreSvcAnnotations:
      metallb.universe.tf/allow-shared-ip: "shared-ss-ip"

and after deploying pravega we should see that all the Segment Store services should be having same LoadBalancerIP and a different port.

In case of an environment which does not allow metallb service to provision the same LoadBalancerIP to services like PKS we should be able to run segment store services on different I/P addresses and same port.

I have verified both Scenarios work

Signed-off-by: prabhaker24 <[email protected]>

…ega/pravega-operator into expose-pravega-on-single-IP

…into expose-pravega-on-single-IP

Signed-off-by: prabhaker24 <[email protected]>

…into expose-pravega-on-single-IP

Signed-off-by: prabhaker24 <[email protected]>

codecov-commenter · 2020-07-27T16:14:48Z

Codecov Report

Merging #431 into master will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #431      +/-   ##
==========================================
+ Coverage   76.11%   76.16%   +0.05%     
==========================================
  Files          14       14              
  Lines        3320     3327       +7     
==========================================
+ Hits         2527     2534       +7     
  Misses        705      705              
  Partials       88       88

Impacted Files	Coverage Δ
pkg/apis/pravega/v1beta1/pravega.go	`97.53% <ø> (ø)`
pkg/controller/pravega/pravega_segmentstore.go	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bea9dfe...bf746a0. Read the comment docs.

Signed-off-by: prabhaker24 <[email protected]>

gaddamas · 2020-07-29T14:30:34Z

pkg/controller/pravega/pravega_segmentstore.go

@@ -491,6 +487,15 @@ func MakeSegmentStoreExternalServices(p *api.PravegaCluster) []*corev1.Service {
 				},
 			},
 		}
+		if strings.EqualFold(p.Spec.Pravega.SegmentStoreExternalTrafficPolicy, "cluster") == true {


change "cluster" to "Cluster" to be consistent with k8s documentation

bourgeoisor

LGTM -- couple comments

bourgeoisor · 2020-07-29T15:24:22Z

README.md

@@ -143,6 +143,8 @@ http://<pravega-name>-pravega-controller.<namespace>:10080/

 Check out the [external access documentation](doc/external-access.md) if your clients need to connect to Pravega from outside Kubernetes.

+Check out the [exposing Segmentstore service on single I/P address ](https://github.com/pravega/pravega-operator/blob/4aa88641c3d5a1d5afbb2b9e628846639fd13290/doc/external-access.md#exposing-segmentstore-service-on-single-ip-address-and-different-ports) if your clients need to connect to Pravega Segment store on the same I/P address from outside Kubernetes.


Isn't IP address the standard way of writing it? (rather than I/P)

bourgeoisor · 2020-07-29T15:26:15Z

charts/pravega/templates/pravega.yaml

@@ -33,6 +33,10 @@ spec:
    {{- if .Values.externalAccess.enabled }}
    controllerServiceAccountName: {{ .Values.serviceAccount.name }}
    segmentStoreServiceAccountName: {{ .Values.serviceAccount.name }}
+    {{- if .Values.segmentStore.service.segmentStoreLoadBalancerIP }}


Is that conditional needed at all? If we remove the conditional and .Values.segmentStore.service.segmentStoreLoadBalancerIP is null / not set, then it's just going to set it as below:

segmentStoreLoadBalancerIP: null`

Which is the same as not setting it (thus, we don't need that conditional)

I tried doing that but I am getting this error spec.pravega.segmentStoreLoadBalancerIP in body must be of type string: "null" so I have kept the condition for now.

doc/external-access.md

SrishT

LGTM other than the minor changes

charts/pravega/README.md

charts/pravega/templates/pravega.yaml

pkg/apis/pravega/v1beta1/pravega.go

doc/external-access.md

SrishT · 2020-07-29T16:57:42Z

doc/external-access.md

+
+By default, Services do not share an IP address, for providing same IP address to all the services we need to set the following configurations while creating the External Service:
+
+1) Provide annotation key as metallb.universe.tf/allow-shared-ip for all the services.


Also the annotation could be highlighted like metallb.universe.tf/allow-shared-ip

Signed-off-by: prabhaker24 <[email protected]>

anishakj

LGTM

prabhaker24 added 12 commits July 21, 2020 10:32

spec changes added

33788cf

Signed-off-by: prabhaker24 <[email protected]>

Merge branch 'expose-pravega-on-single-IP' of https://github.com/prav…

19856fa

…ega/pravega-operator into expose-pravega-on-single-IP

Merge branch 'master' of https://github.com/pravega/pravega-operator …

d426d06

…into expose-pravega-on-single-IP

doc added

f0eada8

Signed-off-by: prabhaker24 <[email protected]>

doc changed

4aa8864

Signed-off-by: prabhaker24 <[email protected]>

changed readme

b108e87

Signed-off-by: prabhaker24 <[email protected]>

changed doc

e4ce1f9

Signed-off-by: prabhaker24 <[email protected]>

chart changes added

6be7fba

Signed-off-by: prabhaker24 <[email protected]>

chart changes added

ea5bafc

Signed-off-by: prabhaker24 <[email protected]>

chart changed

c4bc278

Signed-off-by: prabhaker24 <[email protected]>

Merge branch 'master' of https://github.com/pravega/pravega-operator …

54ac927

…into expose-pravega-on-single-IP

fixed formatting

32628a4

Signed-off-by: prabhaker24 <[email protected]>

Prabhaker24 requested review from gaddamas and pbelgundi July 27, 2020 14:14

prabhaker24 added 2 commits July 27, 2020 22:03

ca bundle added

4b43e13

Signed-off-by: prabhaker24 <[email protected]>

ut added

faea44c

Signed-off-by: prabhaker24 <[email protected]>

Prabhaker24 requested a review from anishakj July 28, 2020 07:28

Prabhaker24 changed the title ~~Expose pravega on single ip~~ Issue 430: Exposing Segmentstore Service on single IP Jul 28, 2020

Prabhaker24 requested review from bourgeoisor and SrishT July 29, 2020 07:54

gaddamas approved these changes Jul 29, 2020

View reviewed changes

bourgeoisor approved these changes Jul 29, 2020

View reviewed changes

SrishT approved these changes Jul 29, 2020

View reviewed changes

addressed comments

bf746a0

Signed-off-by: prabhaker24 <[email protected]>

anishakj approved these changes Jul 30, 2020

View reviewed changes

anishakj merged commit d0218ba into master Jul 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 430: Exposing Segmentstore Service on single IP #431

Issue 430: Exposing Segmentstore Service on single IP #431

Prabhaker24 commented Jul 27, 2020 •

edited

Loading

codecov-commenter commented Jul 27, 2020 •

edited

Loading

gaddamas Jul 29, 2020

Prabhaker24 Jul 29, 2020

bourgeoisor left a comment

bourgeoisor Jul 29, 2020

Prabhaker24 Jul 29, 2020

bourgeoisor Jul 29, 2020

Prabhaker24 Jul 29, 2020 •

edited

Loading

SrishT left a comment

SrishT Jul 29, 2020

Prabhaker24 Jul 29, 2020

anishakj left a comment

		@@ -143,6 +143,8 @@ http://<pravega-name>-pravega-controller.<namespace>:10080/

		Check out the [external access documentation](doc/external-access.md) if your clients need to connect to Pravega from outside Kubernetes.

		Check out the [exposing Segmentstore service on single I/P address ](https://github.com/pravega/pravega-operator/blob/4aa88641c3d5a1d5afbb2b9e628846639fd13290/doc/external-access.md#exposing-segmentstore-service-on-single-ip-address-and-different-ports) if your clients need to connect to Pravega Segment store on the same I/P address from outside Kubernetes.


		By default, Services do not share an IP address, for providing same IP address to all the services we need to set the following configurations while creating the External Service:

		1) Provide annotation key as metallb.universe.tf/allow-shared-ip for all the services.

Issue 430: Exposing Segmentstore Service on single IP #431

Issue 430: Exposing Segmentstore Service on single IP #431

Conversation

Prabhaker24 commented Jul 27, 2020 • edited Loading

Change log description

Purpose of the change

What the code does

How to verify it

codecov-commenter commented Jul 27, 2020 • edited Loading

Codecov Report

gaddamas Jul 29, 2020

Choose a reason for hiding this comment

Prabhaker24 Jul 29, 2020

Choose a reason for hiding this comment

bourgeoisor left a comment

Choose a reason for hiding this comment

bourgeoisor Jul 29, 2020

Choose a reason for hiding this comment

Prabhaker24 Jul 29, 2020

Choose a reason for hiding this comment

bourgeoisor Jul 29, 2020

Choose a reason for hiding this comment

Prabhaker24 Jul 29, 2020 • edited Loading

Choose a reason for hiding this comment

SrishT left a comment

Choose a reason for hiding this comment

SrishT Jul 29, 2020

Choose a reason for hiding this comment

Prabhaker24 Jul 29, 2020

Choose a reason for hiding this comment

anishakj left a comment

Choose a reason for hiding this comment

Prabhaker24 commented Jul 27, 2020 •

edited

Loading

codecov-commenter commented Jul 27, 2020 •

edited

Loading

Prabhaker24 Jul 29, 2020 •

edited

Loading