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 2021/07/15 11:53:29 UTC

[GitHub] [pulsar-helm-chart] valeriano-manassero opened a new pull request #138: automate initialize

valeriano-manassero opened a new pull request #138:
URL: https://github.com/apache/pulsar-helm-chart/pull/138


   Fixes #<xyz>
   
   ### Motivation
   
   Having `initialize` value that must be manually set is prone to human error, there is a risk of running init again forcing the need of a rollback.
   
   ### Modifications
   
   Using Helm capability to understand if it's an install or upgrade we can avoid to launch init when not needed.
   
   ### 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.

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar-helm-chart] lhotari commented on pull request #138: automate initialize

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


   @valeriano-manassero Thank you for the contribution. Would it be also possible to update README.md in this PR and remove the instructions of using `--set initialize=true` when doing the installation?


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

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar-helm-chart] lhotari commented on a change in pull request #138: automate initialize

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



##########
File path: charts/pulsar/templates/bookkeeper-cluster-initialize.yaml
##########
@@ -16,7 +16,7 @@
 # specific language governing permissions and limitations
 # under the License.
 #
-{{- if .Values.initialize }}
+{{- if .Release.IsInstall }}

Review comment:
       I'd suggest keeping backwards compatibility. 
   ```suggestion
   {{- if or .Release.IsInstall .Values.initialize }}
   ```
   The benefit of this is that the metadata initialization job can be created on demand when needed. I have been using this in test environments. It's possible to clear the persistent state of the cluster by scaling stateful sets to 0, deleting PVCs and doing a deployment with `--set initialize=true`. 

##########
File path: charts/pulsar/templates/pulsar-cluster-initialize.yaml
##########
@@ -17,7 +17,7 @@
 # under the License.
 #
 
-{{- if .Values.initialize }}
+{{- if .Release.IsInstall }}

Review comment:
       ```suggestion
   {{- if or .Release.IsInstall .Values.initialize }}
   ```
   

##########
File path: charts/pulsar/values.yaml
##########
@@ -34,9 +34,6 @@ clusterDomain: cluster.local
 ### Global Settings
 ###
 
-## Set to true on install

Review comment:
       please revert this change to keep backwards compatibility.




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

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar-helm-chart] valeriano-manassero commented on pull request #138: automate initialize

Posted by GitBox <gi...@apache.org>.
valeriano-manassero commented on pull request #138:
URL: https://github.com/apache/pulsar-helm-chart/pull/138#issuecomment-918893207


   This is going to be merged?


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

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar-helm-chart] valeriano-manassero commented on pull request #138: automate initialize

Posted by GitBox <gi...@apache.org>.
valeriano-manassero commented on pull request #138:
URL: https://github.com/apache/pulsar-helm-chart/pull/138#issuecomment-1005708870


   > @valeriano-manassero Thank you for the contribution. Would it be also possible to update README.md in this PR and remove the instructions of using `--set initialize=true` when doing the installation?
   
   done


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

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar-helm-chart] lhotari merged pull request #138: automate initialize

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


   


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

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

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