You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/08/13 18:23:48 UTC

[GitHub] [pulsar-helm-chart] toneill818 opened a new pull request #54: Ingress optional hostname

toneill818 opened a new pull request #54:
URL: https://github.com/apache/pulsar-helm-chart/pull/54


   Fixes #50 
   
   ### Motivation
   The host option is not required to setup an ingress, so I made it an optional value
   ### Modifications
   
   *Describe the modifications you've done.*
   Made setting the host optional.
   
   ### Verifying this change
   
   - [ ] Make sure that the change passes the CI checks.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-helm-chart] Skaronator commented on pull request #54: Ingress optional hostname

Posted by GitBox <gi...@apache.org>.
Skaronator commented on pull request #54:
URL: https://github.com/apache/pulsar-helm-chart/pull/54#issuecomment-678109920


   I think it would be much better to use the offical helm approach for defining ingress. The host is basically a array which can be empty (eg. what you want).
   
   You can do `helm create test` and then look at the created ingress at test/template/ingress.yaml. It looks like that:
   
   ```
   {{- if .Values.ingress.enabled -}}
   {{- $fullName := include "test.fullname" . -}}
   {{- $svcPort := .Values.service.port -}}
   {{- if semverCompare ">=1.14-0" .Capabilities.KubeVersion.GitVersion -}}
   apiVersion: networking.k8s.io/v1beta1
   {{- else -}}
   apiVersion: extensions/v1beta1
   {{- end }}
   kind: Ingress
   metadata:
     name: {{ $fullName }}
     labels:
       {{- include "test.labels" . | nindent 4 }}
     {{- with .Values.ingress.annotations }}
     annotations:
       {{- toYaml . | nindent 4 }}
     {{- end }}
   spec:
     {{- if .Values.ingress.tls }}
     tls:
       {{- range .Values.ingress.tls }}
       - hosts:
           {{- range .hosts }}
           - {{ . | quote }}
           {{- end }}
         secretName: {{ .secretName }}
       {{- end }}
     {{- end }}
     rules:
       {{- range .Values.ingress.hosts }}
       - host: {{ .host | quote }}
         http:
           paths:
             {{- range .paths }}
             - path: {{ . }}
               backend:
                 serviceName: {{ $fullName }}
                 servicePort: {{ $svcPort }}
             {{- end }}
       {{- end }}
     {{- end }}
   ```
   And the values.yaml looks like that:
   
   ```
   ingress:
     enabled: false
     annotations: {}
       # kubernetes.io/ingress.class: nginx
       # kubernetes.io/tls-acme: "true"
     hosts:
       - host: chart-example.local
         paths: []
     tls: []
     #  - secretName: chart-example-tls
     #    hosts:
     #      - chart-example.local
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-helm-chart] sijie merged pull request #54: Ingress optional hostname

Posted by GitBox <gi...@apache.org>.
sijie merged pull request #54:
URL: https://github.com/apache/pulsar-helm-chart/pull/54


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-helm-chart] toneill818 commented on pull request #54: Ingress optional hostname

Posted by GitBox <gi...@apache.org>.
toneill818 commented on pull request #54:
URL: https://github.com/apache/pulsar-helm-chart/pull/54#issuecomment-679967736


   @Skaronator That solution still requires a host to be defined as it loops over the hosts list. The goal of this PR was to make the host an optional input. The solution you proposed would not allow for only paths to be defined for an ingress. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-helm-chart] sijie merged pull request #54: Ingress optional hostname

Posted by GitBox <gi...@apache.org>.
sijie merged pull request #54:
URL: https://github.com/apache/pulsar-helm-chart/pull/54


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org