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 running hooks in parallel #349

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
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
68 changes: 68 additions & 0 deletions hips/hip-0019.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
---
hip: 0019
title: "Enable Running Hooks in Parallel"
authors: [ "Michael Morris <[email protected]>" ]
created: "2024-06-06"
type: "feature"
status: "draft"
---

## Abstract

Helm hooks are currently executed serially. In many cases there will be no reason why they cannot be run in parallel. Running hooks in parallel can significantly improve install/upgrade times

## Motivation

Decrease install/upgrade times by executing hooks in parallel

## Specification

Add a new flag to the install, upgrade, rollback and uninstall commands to enable parallel behavior for hook execution. The flag will be of integer type and will allow the user to specify the maximum number of hooks that can be executed in parallel.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider the different user roles for a Helm user. The person installing a chart may have little knowledge of how the package works. Installers of a chart aren't expected to know the business logic of the application. Instead, that knowledge is held by someone distributing the application.

Consider a classic package management situation like installing postgresql on a linux server. The one installing the package may not know where all the files need to live on the operating system, what configuration files need to go where, or the way this app should be added to startup as a daemon. Without having all that knowledge they are able to use the application.

Someone using the app may likely not know if hooks can be run in parallel. The one who would know that is the developer of the chart.

Can we bake this into the chart metadata so the responsibility is on the side of the chart developer instead of the chart user?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mattfarina
Proposed solution updated to address comment. Please take a look when you get time and let me know if there are any further comments


The flag shall be optional. If not set or set to "0" or "1", hook execution shall be done serially. If set to a value greater than "1", hook execution shall be done in parallel, subject to not exceeding the given value in the number of hooks executed in parallel.

Only hooks of the same weight shall be executed in parallel.

## Rationale

While it may be desirable to run hooks in parallel, particularly those of the same weight, doing so by default may cause issues for users depending on the existing serial behavior. Therefore it is proposed to keep the existing serial behavior by default and enable parallel behavior through the use of a flag.

Making the flag an integer rather than boolean allows the user to possibility to restrict the number of parallel processes where resource constraints are a concern.

Allowing hooks of different weights to be executed in parallel is probably not consistent with the documented behavior of lower weight hooks being executed before higher weighted hooks and, therefore only hooks of the same weight shall be executed in parallel

## Backwards compatibility

Proposed solution is fully backwards compatible with no changes in default behavior.

## Security implications

None

## How to teach this

Document in command line help and https://helm.sh/docs/topics/charts_hooks/.

## Reference implementation

PR submitted some time ago: https://github.com/helm/helm/pull/11804
May need to be updated depending on decisions taken on what is proposed here.

## Rejected ideas

Why certain ideas that were brought while discussing this HIP were not
ultimately pursued.

## Open issues

|Issue |Proposal|Decision|
|-----------------------------------------------------------------------------------------|--------|--------|
|Should we allow hooks to be run in parallel |Yes | |
|Should the default behaviour continue to be to execute in serial |Yes | |
|Is there benefit in allowing the user limit the max number of hooks executing in parallel|Yes | |
|Should only hooks of the same weight be executed in parallel |Yes | |


## References

There exists an older closed HIP on hook parallelism, but its focus was on test hooks: https://github.com/helm/community/pull/165