-
-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable extra post-install and post-upgrade helm-hook jobs for environment-specific config #265
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great PR that can resolve a lot of automation use cases.
Very clear implementation.
Thanks!
8dc6320
to
96640ac
Compare
Rebased on master |
fc806b1
to
bc7646d
Compare
e71d7ba
to
2567948
Compare
2567948
to
852ab76
Compare
852ab76
to
454829b
Compare
d6fc648
to
b7051a1
Compare
b7051a1
to
5bff3ce
Compare
Well that was nice. Adding unit tests caught a few issues with this PR. I think we need unit tests for env and volumes before merging this. |
775be16
to
a8cff78
Compare
rebased and squashed into a logical set of commits. |
Like other jobs, the extra_hooks jobs include: annotations, securityContext, pullSecrets, st2client config, envFromSecrets, resources, dnsPolicy/dnsConfig, nodeSelector, affinity, tolerations, init containers, and packs volumes. add securityContext to extra_hooks jobs
use range over command
a8cff78
to
c24a4c7
Compare
Rebased and added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
I think it is worth a release after merging this PR and looking at how many features already accumulated.
# Advanced: Add extra Helm hook Jobs | ||
# These hook jobs will use the same settings (eg image, annotations, pod placement) as the other jobs. | ||
# They will have st2 cli configured, st2.conf files, and packs volumes mounted. | ||
# See available hooks list: https://helm.sh/docs/topics/charts_hooks/#the-available-hooks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is powerful, but I think the use case examples would be nice to include somewhere. Otherwise, very few would know about it and use it.
Perhaps a Readme section describing this feature with some examples?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. I will add a README section.
Also good to know that new tests already caught some issues 👍 |
K. README updated in e4b72dc |
I'll merge now. We can revise the README in a follow-up if needed. |
Resolves #234
There is a lot of boilerplate for custom StackStorm hook-jobs. That boilerplate is likely to become out-of-date as we refine the chart, so managing that boilerplate within this chart alongside our other jobs would be ideal.
Since there is so much chart-specific boilerplate, parent charts would have to depend on a lot of implementation details making them very brittle. Parent charts are much better suited to jobs that don't need access to most of the packs, configs, configmaps, and secrets that this chart provides.
These hook-jobs can be used for st2 installation-specific jobs like: