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/07/30 18:57:06 UTC

[GitHub] [pulsar-helm-chart] toneill818 opened a new pull request #48: Allow Grafana to work with a reverse proxy

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


   ### Motivation
   
   Allow Grafana to be served from a sub path.  
   
   ### Modifications
   
   - Added a config map to add extra environment variables to the grafana deployment. As the grafana image adds new features that require environment variables, this can be used to set them.
   - Bumped the grafana image to allow a reverse proxy
   - removed ingress annotations as they are specific to nginx, and to match all the other ingresses
   - bumped the chart version as per the README 
   
   
   Example values:
   ```
   grafana:
     configData:
       GRAFANA_ROOT_URL: /pulsar/grafana
       GRAFANA_SERVE_FROM_SUB_PATH: "true"
     ingress:
         enabled: true
         port: 3000
         path: "/pulsar/grafana/?(.*)"
         annotations:
           nginx.ingress.kubernetes.io/rewrite-target: /$1
   ```
   


----------------------------------------------------------------
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 commented on a change in pull request #48: Allow Grafana to work with a reverse proxy

Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #48:
URL: https://github.com/apache/pulsar-helm-chart/pull/48#discussion_r469056203



##########
File path: charts/pulsar/Chart.yaml
##########
@@ -21,7 +21,7 @@ apiVersion: v1
 appVersion: "2.6.0"
 description: Apache Pulsar Helm chart for Kubernetes
 name: pulsar
-version: 2.6.0
+version: 2.6.1

Review comment:
       Yes we don't need to align the helm chart release with pulsar release.




----------------------------------------------------------------
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 a change in pull request #48: Allow Grafana to work with a reverse proxy

Posted by GitBox <gi...@apache.org>.
Skaronator commented on a change in pull request #48:
URL: https://github.com/apache/pulsar-helm-chart/pull/48#discussion_r469159385



##########
File path: charts/pulsar/Chart.yaml
##########
@@ -21,7 +21,7 @@ apiVersion: v1
 appVersion: "2.6.0"
 description: Apache Pulsar Helm chart for Kubernetes
 name: pulsar
-version: 2.6.0
+version: 2.6.1

Review comment:
       Not a huge fan of that if when we want to follow the semantic versioning for this helm chart but still better than nothing. Thanks for the new release ;)




----------------------------------------------------------------
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 commented on a change in pull request #48: Allow Grafana to work with a reverse proxy

Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #48:
URL: https://github.com/apache/pulsar-helm-chart/pull/48#discussion_r464652144



##########
File path: charts/pulsar/Chart.yaml
##########
@@ -21,7 +21,7 @@ apiVersion: v1
 appVersion: "2.6.0"
 description: Apache Pulsar Helm chart for Kubernetes
 name: pulsar
-version: 2.6.0
+version: 2.6.1

Review comment:
       Since the chart is for pulsar 2.6.0 and we don't release pulsar 2.6.1 yet, I would suggest not bumping the version here. We can use a naming convention like `<pulsar-version>-<minor-version>` for versioning the helm chart. How does that sound?

##########
File path: charts/pulsar/values.yaml
##########
@@ -856,16 +856,16 @@ grafana:
     targetPort: 3000
     annotations: {}
   plugins: []
+  ## Grafana configMap
+  ## templates/grafana-configmap.yaml
+  ##
+  configData: {}
   ## Grafana ingress
   ## templates/grafana-ingress.yaml
   ##
   ingress:
     enabled: false
-    annotations:

Review comment:
       why did you remove the default values?




----------------------------------------------------------------
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 a change in pull request #48: Allow Grafana to work with a reverse proxy

Posted by GitBox <gi...@apache.org>.
Skaronator commented on a change in pull request #48:
URL: https://github.com/apache/pulsar-helm-chart/pull/48#discussion_r469159385



##########
File path: charts/pulsar/Chart.yaml
##########
@@ -21,7 +21,7 @@ apiVersion: v1
 appVersion: "2.6.0"
 description: Apache Pulsar Helm chart for Kubernetes
 name: pulsar
-version: 2.6.0
+version: 2.6.1

Review comment:
       Not a huge fan of that. It would make it impossible to use semantic versioning for this helm chart specific... but still better than nothing. Thanks for the new release ;)




----------------------------------------------------------------
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 a change in pull request #48: Allow Grafana to work with a reverse proxy

Posted by GitBox <gi...@apache.org>.
Skaronator commented on a change in pull request #48:
URL: https://github.com/apache/pulsar-helm-chart/pull/48#discussion_r469037766



##########
File path: charts/pulsar/Chart.yaml
##########
@@ -21,7 +21,7 @@ apiVersion: v1
 appVersion: "2.6.0"
 description: Apache Pulsar Helm chart for Kubernetes
 name: pulsar
-version: 2.6.0
+version: 2.6.1

Review comment:
       @sijie Thats what the "appVersion" Value in the Chart.yaml is for. The "version" key is just the version of the helm chart and is completly separate from the actual application version. And when you follow semantic versioning on your helm chart as well then these version will seperate really quickly.
   
   Just a quick look at the Prometheus Helm Chat: Prometheus itself is at 2.20.1 right now but the helm chart is already at 11.x https://github.com/helm/charts/blob/master/stable/prometheus/Chart.yaml#L3-L4
   
   It would be cool to have more frequent updates of this helm chart and not just wait for pulsar updates to release at the same time.




----------------------------------------------------------------
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 a change in pull request #48: Allow Grafana to work with a reverse proxy

Posted by GitBox <gi...@apache.org>.
toneill818 commented on a change in pull request #48:
URL: https://github.com/apache/pulsar-helm-chart/pull/48#discussion_r464662691



##########
File path: charts/pulsar/values.yaml
##########
@@ -856,16 +856,16 @@ grafana:
     targetPort: 3000
     annotations: {}
   plugins: []
+  ## Grafana configMap
+  ## templates/grafana-configmap.yaml
+  ##
+  configData: {}
   ## Grafana ingress
   ## templates/grafana-ingress.yaml
   ##
   ingress:
     enabled: false
-    annotations:

Review comment:
       They get merged with other values if a user provides annotations. This also matches the other ingress values. So if I am not using nginx ingress, it still attaches the nginx ingress annotations. 




----------------------------------------------------------------
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 commented on a change in pull request #48: Allow Grafana to work with a reverse proxy

Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #48:
URL: https://github.com/apache/pulsar-helm-chart/pull/48#discussion_r469056007



##########
File path: charts/pulsar/Chart.yaml
##########
@@ -21,7 +21,7 @@ apiVersion: v1
 appVersion: "2.6.0"
 description: Apache Pulsar Helm chart for Kubernetes
 name: pulsar
-version: 2.6.0
+version: 2.6.1

Review comment:
       I understand the difference between appVersion and version. My recommendation is to add another version. e.g. `2.6.1-0` this means it is a helm chart released for pulsar `2.6.1`.




----------------------------------------------------------------
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 #48: Allow Grafana to work with a reverse proxy

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


   


----------------------------------------------------------------
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 a change in pull request #48: Allow Grafana to work with a reverse proxy

Posted by GitBox <gi...@apache.org>.
toneill818 commented on a change in pull request #48:
URL: https://github.com/apache/pulsar-helm-chart/pull/48#discussion_r464663237



##########
File path: charts/pulsar/Chart.yaml
##########
@@ -21,7 +21,7 @@ apiVersion: v1
 appVersion: "2.6.0"
 description: Apache Pulsar Helm chart for Kubernetes
 name: pulsar
-version: 2.6.0
+version: 2.6.1

Review comment:
       I can do that, the appVersion will match the pulsar version the chart is running. The helm version and pulsar version are different. Do you want them the same so its easy to know what version you are running if you use helm? 




----------------------------------------------------------------
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