Skip to content
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

Add HIP for hooks with equivalent weight run in parallel #288

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 93 additions & 0 deletions hips/hip-9999.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
---
hip: 9999
title: "Hook Parallelism"
authors: [ "Jeff van Dam <[email protected]>" ]
created: "2023-02-28"
type: "feature "
status: "draft"

---

## Abstract
This feature would allow for hooks to be run in parallel instead of serially
only for lifecycle hooks e.g pre-update, post-delete, pre-rollback, etc. Since
Helm does not do this already this will save a significant amount of time
during release operations with many hooks of the same weight.

This feature will NOT work for test hooks!

## Motivation
The motivation behind this feature is to save time during release lifecycle
operations. Many hooks can run in parallel as they are not in any way tied to
each other, so these hooks can be given the same weight. However even hooks
marked with the same weight are not ran in parallel in helm. This can lead to a
significant amount of time during the install/upgrade/rollback/uninstall,
running hooks in order. Many hooks may only take a few milliseconds to run,
however helm seems to have a built in timer which it seems to use to fire each
hook in order, check that it completes, run the next hook. Many minutes are
lost because of this.

## Rationale
The proposed design is to add a flag called "hook-parallelism", which defaults
to 1. When hooks are run they are then run in go routines, up to, but not
exceeding the value of the flag concurrently. This will allow for users to only
use the flag if they require their hooks to run in parallel. Not including the
flag, or including the flag with a value of 1 or less will be the same as
running serially.

This feature will not allow test hooks to be run in parallel because it is up
to the Application Distributor to write tests that they think should run in
parallel or serially.

## Specification
The flag (hook-parallelism) will be added to all applicable commands (install,
rollback, uninstall, upgrade), and will allow for any hooks attached to those
commands to be run in parallel when their weight is equivalent. Not including a
flag will have the hooks run serially. The actual implementation will use a
semaphore based on the value of the flag to limit the number of hooks being
run. While the number of go routines run will be equal to the number of hooks
(sorted by weight), only the value of the flag number of go routines will be
run at the same time. This will allow the user flexibility in their usage in
relation to their system constraints.

## Backwards compatibility
This should have no backward compatibility issues. Not including the flag will
have the value default to 1. This will only allow one hook to run at a time,
and the user should notice no difference in the execution of their hooks.

## Security implications
A malicious user could provide an unreasonably high number for the value of the
hook parallelism flag. This would set the length of the semaphore. This should
not be something that should affect the normal user.

## How to teach this
A good example would be to have two separate test hooks written. Run initially
and watch one pod spin up and run, then a second after the first has completed.
Next the flag could be added, and shown that both test pods are spun up at the
same time. It might be helpful to mention a general estimate of how many hooks
you want to run in parallel compared to the number of hooks you have associated
with a command.

## Reference implementation
Currently there is an open Github Pull Request [here](https://github.com/helm/helm/pull/11804) which was based on
[this review](https://github.com/helm/helm/pull/8946) that was based on [this review](https://github.com/helm/helm/pull/7792) which was made from [this issue](https://github.com/helm/helm/issues/7763).

The reviews that this HIP is based on includes the abilty to parallelize test
hooks which is not the case for this HIP.


## Rejected ideas
This HIP is based on [another HIP](https://github.com/helm/community/pull/165) which stalled after several rounds of reviews
pertaining to running test hooks in parallel which this HIP does not want to
accomplish.


## Open issues
N/A

## References
- https://github.com/helm/community/pull/165
- https://github.com/helm/helm/pull/8946
- https://github.com/helm/helm/pull/7792
- https://github.com/helm/helm/issues/7763
- https://github.com/helm/helm/pull/11804