camunda-community-hub / zeebe-cluster-helm

Base Zeebe Cluster HELM Chart
Apache License 2.0
16 stars 31 forks source link

Configure port names via values.yaml to support Istio Port Selection #37

Closed andylholloway closed 4 years ago

andylholloway commented 4 years ago

Hi I recently deployed Zeebe to a kubernetes cluster that uses the Istio Service Mesh.

To configure Port Selection (https://istio.io/docs/ops/configuration/traffic-management/protocol-selection/) the port names should be prefixed with the protocol e.g. grpc-gateway.

At the moment I am pulling down the helm chart, expanding the charts and modifying the port names.

Could the port names be configurable via the values.yaml file?

Example: Service.yaml

    - port: {{ .Values.service.http.port }}
      protocol: TCP
      name: http   # change to {{ .Values.service.http.name }}
    - port: {{ .Values.service.gateway.port }}
      protocol: TCP
      name: gateway  # change to {{ .Values.service.gateway.name }}

Statefulset.yaml

ports:
        - containerPort: 9600
          name: http    # change to {{ .Values.service.http.name }}
        - containerPort: 26500
          name: gateway  # change to {{ .Values.service.gateway.name }}
        - containerPort: 26501
          name: command  # change to {{ .Values.service.command.name }}
        - containerPort: 26502
          name: internal  # change to {{ .Values.service.internal.name }}

values.yaml

service:
  type: ClusterIP
  http:
    port: 9600
    name: http-web
  gateway:
    port: 26500
    name: grpc-gateway
  command:
    port: 26501
    name: tcp-command
  internal:
    port: 26502
    name: tcp-internal

Thanks Andy

Zelldon commented 4 years ago

Hey @andylholloway,

thanks for opening this issue, sounds reasonable for me.

Probably it would make sense to have this as default names, or?

Are you willing in providing a PR for that?

Greets Chris

andylholloway commented 4 years ago

Hi @Zelldon

I'd be happy to provide a PR for this.

I was thinking of keeping the existing port names as defaults and only overriding a port name if a port name is provided in the values.yaml. Is that how you see it?

Zelldon commented 4 years ago

@npepinpe any opinion on this?

npepinpe commented 4 years ago

Is the automatic protocol detection only detecting HTTP and HTTP/2? I was under the impression they could detect most protocols, but maybe not. :disappointed_relieved:

@andylholloway's proposal sounds good - keep default port names and overwrite only if provided. :+1:

andylholloway commented 4 years ago

AFAIK Istio can auto-detect HTTP and HTTP/2, but will default to TCP when it cannot determine the protocol.

andylholloway commented 4 years ago

@Zelldon , @npepinpe I have made the changes in a local branch, but I'm not sure how to push my changes and create a PR.

Could you tell me what steps I need to take to create a PR?

Thanks Andy

npepinpe commented 4 years ago

For contributors, you would fork the repository, then push the branch to your fork. You can then go to Pull Requests and open a PR against our repository's master branch.

See here on how to create and maintain forks: https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/working-with-forks

And here on how to open a PR from your own fork: https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/committing-changes-to-a-pull-request-branch-created-from-a-fork

Let us know if you need anything else!

npepinpe commented 4 years ago

Closed by #39