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

Issue 430: Exposing Segmentstore Service on single IP #431

Merged
merged 15 commits into from
Jul 30, 2020
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


### Scale a Pravega cluster

You can scale Pravega components independently by modifying their corresponding field in the Pravega resource spec. You can either `kubectl edit` the cluster or `kubectl patch` it. If you edit it, update the number of replicas for BookKeeper, Controller, and/or Segment Store and save the updated spec.
Expand Down
8 changes: 7 additions & 1 deletion charts/pravega-operator/templates/crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ spec:
conversionReviewVersions: ["v1beta1", "v1alpha1"]
strategy: Webhook
webhookClientConfig:
caBundle: {{ .Values.webhookCert.crt }}
caBundle: {{ .Values.webhookCert.crt }}
service:
name: pravega-webhook-svc
namespace: {{ .Release.Namespace }}
Expand Down Expand Up @@ -79,6 +79,8 @@ spec:
static:
type: object
properties:
caBundle:
type: string
controllerSecret:
type: string
segmentStoreSecret:
Expand Down Expand Up @@ -267,6 +269,10 @@ spec:
additionalProperties:
type: string
type: object
segmentStoreLoadBalancerIP:
type: string
segmentStoreExternalTrafficPolicy:
type: string
status:
type: object
description: PravegaCluster Status defines the observed state of PravegaCluster
Expand Down
2 changes: 2 additions & 0 deletions charts/pravega/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ The following table lists the configurable parameters of the Pravega chart and t
| `segmentStore.resources.limits.memory` | Memory limits for segmentStore | `4Gi` |
| `segmentStore.service.type` | Override the segmentStore service type, if external access is enabled (LoadBalancer/NodePort) | |
| `segmentStore.service.annotations` | Annotations to add to the segmentStore service, if external access is enabled | `{}` |
| `segmentStore.service.segmentStoreLoadBalancerIP` |It is used to provide a Load LoadBalancerIP | |
SrishT marked this conversation as resolved.
Show resolved Hide resolved
| `segmentStore.service.segmentStoreExternalTrafficPolicy` | It is used to provide segmentStoreExternalTrafficPolicy | |
| `segmentStore.jvmOptions` | JVM Options for segmentStore | `[]` |
| `storage.longtermStorage.type` | Type of long term storage backend to be used (filesystem/ecs/hdfs) | `filesystem` |
| `storage.longtermStorage.filesystem.pvc` | Name of the pre-created PVC, if long term storage type is filesystem | `pravega-tier2` |
Expand Down
8 changes: 6 additions & 2 deletions charts/pravega/templates/pravega.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ metadata:
labels:
{{ include "pravega.commonLabels" . | indent 4 }}
spec:
{{- if .Values.tls }}
{{- if .Values.tls }}
SrishT marked this conversation as resolved.
Show resolved Hide resolved
tls:
static:
controllerSecret: {{ .Values.tls.secret.controller }}
Expand All @@ -33,6 +33,10 @@ spec:
{{- if .Values.externalAccess.enabled }}
controllerServiceAccountName: {{ .Values.serviceAccount.name }}
segmentStoreServiceAccountName: {{ .Values.serviceAccount.name }}
{{- if .Values.segmentStore.service.segmentStoreLoadBalancerIP }}
Copy link
Contributor

Choose a reason for hiding this comment

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

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)

Copy link
Contributor Author

@Prabhaker24 Prabhaker24 Jul 29, 2020

Choose a reason for hiding this comment

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

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.

segmentStoreLoadBalancerIP: {{ .Values.segmentStore.service.segmentStoreLoadBalancerIP }}
{{- end }}
segmentStoreExternalTrafficPolicy: {{ .Values.segmentStore.service.segmentStoreExternalTrafficPolicy }}
{{- if .Values.controller.service.type }}
controllerExtServiceType: {{ .Values.controller.service.type }}
{{- end }}
Expand All @@ -45,7 +49,7 @@ spec:
{{- end }}
{{- if .Values.segmentStore.service.annotations }}
segmentStoreSvcAnnotations:
{{ toYaml .Values.segmentStore.service.annotations | indent 6 }}
{{ toYaml .Values.segmentStore.service.annotations | indent 6}}
SrishT marked this conversation as resolved.
Show resolved Hide resolved
{{- end }}
{{- end }}
image:
Expand Down
2 changes: 2 additions & 0 deletions charts/pravega/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ segmentStore:
## used to override the service type for segmentStore
type:
annotations: {}
segmentStoreLoadBalancerIP:
segmentStoreExternalTrafficPolicy: Local
jvmOptions: ["-Xmx2g", "-XX:MaxDirectMemorySize=2g"]

storage:
Expand Down
6 changes: 6 additions & 0 deletions deploy/crds/crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ spec:
static:
type: object
properties:
caBundle:
type: string
controllerSecret:
type: string
segmentStoreSecret:
Expand Down Expand Up @@ -264,6 +266,10 @@ spec:
additionalProperties:
type: string
type: object
segmentStoreLoadBalancerIP:
type: string
segmentStoreExternalTrafficPolicy:
type: string
status:
type: object
description: PravegaCluster Status defines the observed state of PravegaCluster
Expand Down
28 changes: 28 additions & 0 deletions doc/external-access.md
Original file line number Diff line number Diff line change
Expand Up @@ -238,3 +238,31 @@ IP: 10.100.200.183
LoadBalancer Ingress: 10.247.108.104
. . .
```
# Exposing Segmentstore Service on single IP address and Different ports

For Exposing SegmentStoreservices on the same I/P address we will use MetalLB,
SrishT marked this conversation as resolved.
Show resolved Hide resolved
MetalLB hooks into Kubernetes cluster, and provides a network load-balancer implementation. In short, it allows to create Kubernetes services of type “LoadBalancer” in clusters that don’t run on a cloud provider and thus cannot simply hook into paid products to provide load-balancers.

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


2) All the services which want to share the IP address need to have the same value for the above annotation, for example "shared-ss-ip".

3) The port for all the services should be different

4) All the services should use External Traffic Policy as Cluster

5) Finally, we need to provide the I/P address that we want our service to provide to the segment store pod as spec.loadBalancerIP while creating the service

To enable this we need to provide segmentStoreSvcAnnotations, segmentStoreLoadBalancerIP, segmentStoreExternalTrafficPolicy in the manifest as shown below

Example:
```
pravega:
. . .
segmentStoreLoadBalancerIP: "10.243.39.103"
segmentStoreExternalTrafficPolicy: "cluster"
segmentStoreSvcAnnotations:
metallb.universe.tf/allow-shared-ip: "shared-ss-ip"
```
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
4d63.com/gochecknoinits v0.0.0-20200108094044-eb73b47b9fc4 // indirect
github.com/alecthomas/gocyclo v0.0.0-20150208221726-aa8f8b160214 // indirect
github.com/alexkohler/nakedret v1.0.0 // indirect
github.com/gordonklaus/ineffassign v0.0.0-20200309095847-7953dde2c7bf // indirect
github.com/hashicorp/go-version v1.1.0
github.com/jgautheron/goconst v0.0.0-20200227150835-cda7ea3bf591 // indirect
github.com/mdempsky/unconvert v0.0.0-20200228143138-95ecdbfc0b5f // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,8 @@ github.com/gophercloud/gophercloud v0.6.0 h1:Xb2lcqZtml1XjgYZxbeayEemq7ASbeTp09m
github.com/gophercloud/gophercloud v0.6.0/go.mod h1:GICNByuaEBibcjmjvI7QvYJSZEbGkcYwAR7EZK2WMqM=
github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY=
github.com/gopherjs/gopherjs v0.0.0-20191106031601-ce3c9ade29de/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY=
github.com/gordonklaus/ineffassign v0.0.0-20200309095847-7953dde2c7bf h1:vc7Dmrk4JwS0ZPS6WZvWlwDflgDTA26jItmbSj83nug=
github.com/gordonklaus/ineffassign v0.0.0-20200309095847-7953dde2c7bf/go.mod h1:cuNKsD1zp2v6XfE/orVX2QE1LC+i254ceGcVeDT3pTU=
github.com/gorilla/context v1.1.1/go.mod h1:kBGZzfjB9CEq2AlWe17Uuf7NDRt0dE0s8S51q0aT7Yg=
github.com/gorilla/handlers v0.0.0-20150720190736-60c7bfde3e33/go.mod h1:Qkdc/uu4tH4g6mTK6auzZ766c4CA0Ng8+o/OAirnOIQ=
github.com/gorilla/mux v1.6.2/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs=
Expand Down
6 changes: 6 additions & 0 deletions pkg/apis/pravega/v1beta1/pravega.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,12 @@ type PravegaSpec struct {

// Annotations to be added to the external service
SegmentStoreServiceAnnotations map[string]string `json:"segmentStoreSvcAnnotations"`

// Specifying this I/P would insure we use same I/P address for all the ss services
SrishT marked this conversation as resolved.
Show resolved Hide resolved
SegmentStoreLoadBalancerIP string `json:"segmentStoreLoadBalancerIP,omitempty"`

// SegmentStoreExternalTrafficPolicy defines the ExternalTrafficPolicy it can have cluster or local
SegmentStoreExternalTrafficPolicy string `json:"segmentStoreExternalTrafficPolicy,omitempty"`
}

func (s *PravegaSpec) withDefaults() (changed bool) {
Expand Down
13 changes: 9 additions & 4 deletions pkg/controller/pravega/pravega_segmentstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,20 +450,16 @@ func generateDNSAnnotationForSvc(domainName string, podName string) (dnsAnnotati

func MakeSegmentStoreExternalServices(p *api.PravegaCluster) []*corev1.Service {
var service *corev1.Service

serviceType := getSSServiceType(p)
services := make([]*corev1.Service, p.Spec.Pravega.SegmentStoreReplicas)

for i := int32(0); i < p.Spec.Pravega.SegmentStoreReplicas; i++ {
ssPodName := p.ServiceNameForSegmentStore(i)
annotationMap := p.Spec.Pravega.SegmentStoreServiceAnnotations
annotationValue := generateDNSAnnotationForSvc(p.Spec.ExternalAccess.DomainName, ssPodName)

if annotationValue != "" {
annotationMap = cloneMap(p.Spec.Pravega.SegmentStoreServiceAnnotations)
annotationMap[externalDNSAnnotationKey] = annotationValue
}

service = &corev1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
Expand Down Expand Up @@ -491,6 +487,15 @@ func MakeSegmentStoreExternalServices(p *api.PravegaCluster) []*corev1.Service {
},
},
}
if strings.EqualFold(p.Spec.Pravega.SegmentStoreExternalTrafficPolicy, "cluster") == true {

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

service.Spec.ExternalTrafficPolicy = corev1.ServiceExternalTrafficPolicyTypeCluster
} else {
service.Spec.ExternalTrafficPolicy = corev1.ServiceExternalTrafficPolicyTypeLocal
}
if p.Spec.Pravega.SegmentStoreLoadBalancerIP != "" {
service.Spec.Ports[0].Port = 12345 + i
service.Spec.LoadBalancerIP = p.Spec.Pravega.SegmentStoreLoadBalancerIP
}
services[i] = service
}
return services
Expand Down
19 changes: 19 additions & 0 deletions pkg/controller/pravega/pravega_segmentstore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ var _ = Describe("PravegaSegmentstore", func() {
It("should set external access service type to LoadBalancer", func() {
Ω(p.Spec.ExternalAccess.Type).Should(Equal(corev1.ServiceTypeClusterIP))
})

})
})

Expand Down Expand Up @@ -377,6 +378,24 @@ var _ = Describe("PravegaSegmentstore", func() {
Ω(err).Should(BeNil())
})
})
Context("Create External service with SegmentStoreExternalTrafficPolicy as cluster", func() {
BeforeEach(func() {
p.Spec.Pravega.SegmentStoreExternalTrafficPolicy = "cluster"
})
It("should create external service with ExternalTrafficPolicy type cluster", func() {
svc := pravega.MakeSegmentStoreExternalServices(p)
Ω(svc[0].Spec.ExternalTrafficPolicy).To(Equal(corev1.ServiceExternalTrafficPolicyTypeCluster))
})
})
Context("Create External service with LoadBalancerIP", func() {
BeforeEach(func() {
p.Spec.Pravega.SegmentStoreLoadBalancerIP = "10.240.12.18"
})
It("should create external service with LoadBalancerIP", func() {
svc := pravega.MakeSegmentStoreExternalServices(p)
Ω(svc[0].Spec.LoadBalancerIP).To(Equal("10.240.12.18"))
})
})
Context("Create External service with external service type empty", func() {
BeforeEach(func() {
m := make(map[string]string)
Expand Down