You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "doug-ba (via GitHub)" <gi...@apache.org> on 2024/03/22 22:11:25 UTC
[PR] making .ReleasIsInstall optional for init jobs [pulsar-helm-chart]
doug-ba opened a new pull request, #480:
URL: https://github.com/apache/pulsar-helm-chart/pull/480
Fixes #479
### Motivation
In solutions like ArgoCD which don't actually track helm state, .ReleaseIsInstall always evaluates to true. We should be able to optionally not use .ReleaseIsInstall to determine whether init jobs should be created/removed
### Modifications
makes a new value `useReleaseStatus` which defaults to true.
When `useReleaseStatus` is set to false, the chart relies only on the value of the `initialize` value to determine whether the init jobs should be created.
This PR also changes an `or` in the pulsar-manager-cluster-initialize template to an `and`. With the `or` in place, the value passed at `.Values.job.ttl.enabled` was ignored for all clusters newer than Kubernetes version 1.23.0. This causes the job to be constantly recreated in argocd.
### 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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] making .ReleaseIsInstall optional for init jobs [pulsar-helm-chart]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #480:
URL: https://github.com/apache/pulsar-helm-chart/pull/480#discussion_r1536593606
##########
charts/pulsar/templates/bookkeeper-cluster-initialize.yaml:
##########
@@ -16,7 +16,7 @@
# specific language governing permissions and limitations
# under the License.
#
-{{- if or .Release.IsInstall .Values.initialize }}
+{{- if or (and .Values.useReleaseStatus (or .Release.IsInstall .Values.initialize)) .Values.initialize }}
Review Comment:
It seem that the condition could be simplified
```suggestion
{{- if or (and .Values.useReleaseStatus .Release.IsInstall) .Values.initialize }}
```
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] making .ReleaseIsInstall optional for init jobs [pulsar-helm-chart]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari merged PR #480:
URL: https://github.com/apache/pulsar-helm-chart/pull/480
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] making .ReleaseIsInstall optional for init jobs [pulsar-helm-chart]
Posted by "doug-ba (via GitHub)" <gi...@apache.org>.
doug-ba commented on code in PR #480:
URL: https://github.com/apache/pulsar-helm-chart/pull/480#discussion_r1538070179
##########
charts/pulsar/templates/bookkeeper-cluster-initialize.yaml:
##########
@@ -16,7 +16,7 @@
# specific language governing permissions and limitations
# under the License.
#
-{{- if or .Release.IsInstall .Values.initialize }}
+{{- if or (and .Values.useReleaseStatus (or .Release.IsInstall .Values.initialize)) .Values.initialize }}
Review Comment:
Thanks for the feedback, update pushed.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org