You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by "yekibud (via GitHub)" <gi...@apache.org> on 2023/06/21 16:00:29 UTC
[GitHub] [couchdb-helm] yekibud opened a new pull request, #124: Support lifecycle hooks
yekibud opened a new pull request, #124:
URL: https://github.com/apache/couchdb-helm/pull/124
<!--
Thank you for contributing to couchdb-helm. Before you submit this PR we'd like to
make sure you are aware of the chart technical requirements and best practices:
* https://github.com/helm/charts/blob/master/CONTRIBUTING.md#technical-requirements
* https://github.com/helm/helm/tree/master/docs/chart_best_practices
For a quick overview across what we will look at reviewing your PR, please read
our review guidelines:
* https://github.com/helm/charts/blob/master/REVIEW_GUIDELINES.md
Following our best practices right from the start will accelerate the review process and
help get your PR merged quicker.
When updates to your PR are requested, please add new commits and do not squash the
history. This will make it easier to identify new changes. The PR will be squashed
anyways when it is merged. Thanks.
Please make sure you test your changes before you push them.
-->
#### What this PR does / why we need it:
Adds support for container lifecycle hooks which serve different use cases that helm hooks and can be more reliable for similar use cases (e.g. for changes outside of helm management like a pod or statefulset being deleted).
#### Special notes for your reviewer:
My use case requires a sophisticated postStart hook with variable interpolation from a parent chart, but I figured many users of this chart may not use it as a subchart, so I included the option to pass in simple container hooks via values, as well. I tried to make this opaque to the user so they could just use `.Values.lifecycle` in both cases, but I couldn't figure out how to get helm to differentiate between a template and a values map since both are `map` types. So I wound up supporting both use cases with `.Values.lifecycle` and `.Values.lifecycleTemplate`.
#### Checklist
- [x] Chart Version bumped
- [ ] e2e tests pass
- [x] Variables are documented in the README.md
I get the following error when trying to run `make test`. Not sure if this is an existing issue, but it looks unrelated to the changes in this PR.
```
Error: failed linting and installing charts: failed identifying merge base: must be in a git repository
------------------------------------------------------------------------------------------------------------------------
No chart changes detected.
------------------------------------------------------------------------------------------------------------------------
failed linting and installing charts: failed identifying merge base: must be in a git repository
Removing ct container...
Deleting cluster "chart-testing" ...
Done!
make: *** [Makefile:31: test] Error 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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-helm] yekibud commented on pull request #124: Support lifecycle hooks
Posted by "yekibud (via GitHub)" <gi...@apache.org>.
yekibud commented on PR #124:
URL: https://github.com/apache/couchdb-helm/pull/124#issuecomment-1672369875
> Thanks for the PR @yekibud. I don't see any issues with this so can merge once it's rebased, tests pass etc.
@willholley Thanks. Rebased and updated my branch. I get the same error I reported for `make test` on `main` so I don't think that has to do with my PR.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-helm] willholley commented on pull request #124: Support lifecycle hooks
Posted by "willholley (via GitHub)" <gi...@apache.org>.
willholley commented on PR #124:
URL: https://github.com/apache/couchdb-helm/pull/124#issuecomment-1670838784
Thanks for the PR @yekibud. I don't see any issues with this so can merge once it's rebased, tests pass etc.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-helm] yekibud commented on pull request #124: Support lifecycle hooks
Posted by "yekibud (via GitHub)" <gi...@apache.org>.
yekibud commented on PR #124:
URL: https://github.com/apache/couchdb-helm/pull/124#issuecomment-1670559853
@willholley any feedback? In case the use case of the PR isn't clear, here's a more concrete example:
Let's say you need to iterate through a list of values to create admin users, or some other dynamic process after your couchdb instance is stood up. This PR enables you to do that by providing a template to the statefulset that will allow you to populate those values.
Sound useful?
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org