From 6d420e8cce6fec7faa77e2dedeb261ebdfd1d5bc Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Fri, 6 Dec 2024 20:17:07 +0000 Subject: [PATCH 01/18] start helm hip Signed-off-by: Austin Abro --- hips/hip-0999.md | 63 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 hips/hip-0999.md diff --git a/hips/hip-0999.md b/hips/hip-0999.md new file mode 100644 index 00000000..dd011c47 --- /dev/null +++ b/hips/hip-0999.md @@ -0,0 +1,63 @@ +--- +hip: 9999 +title: "Wait With kstatus" +authors: [ "@austinabro321" ] +created: "2024-12-06" +type: "feature" +status: "draft" +--- + +## Abstract + +Currently the `--wait` flag on `helm install` and `helm upgrade` does not wait for all resources to be fully reconciled, and does not wait for custom resources (CRs) at all. By replacing the current wait logic with kstatus Helm will achieve *better* waits, while simplifying it's code and documentation. + +## Motivation + +Clearly explain why the existing design is inadequate to address the problem +that the HIP solves. + +Certain workflows require CRDs to be ready before continuing to the next step. There is no way to tell Helm to wait for custom resources to be ready, so anyone that has this requirement must write their own logic to wait for their custom resources. + +Certain workflows may want to wait until their resources are fully reconciled before continuing to their next step. For example, Helm waits for all new pods in an upgraded deployment to be ready. However, Helm does not wait for the previous pods in that deployment to be removed. Ensuring only the new pods are available after waiting for an upgrade can come in handy. + +By introducing kstatus we will lower user friction with the `--wait` flag. + +## Rationale + +Describe why particular design decisions were made. + +## Specification + +Describe the syntax and semantics of any new feature. + +## Backwards compatibility + +Describe potential impact and severity on pre-existing code. + +## Security implications + +No security implications + +## How to teach this + +Replace the [existing wait documentation](https://helm.sh/docs/intro/using_helm/) by explaining that we use kstatus and pointing to the [kstatus documentation](https://github.com/kubernetes-sigs/cli-utils/blob/master/pkg/kstatus/README.md). This comes with the added benefit of not needed to maintain Helm specific wait documentation. A separate team has already written extensive documentation on how the wait logic of kstatus works. + +## Reference implementation + +Link to any existing implementation and details about its state, e.g. +proof-of-concept. + +Here is an implementation of waiting for resources using kstatus in a project I maintain called Zarf. I will replace this section with a draft PR showing an example implementation in Helm once I have feedback that this HIP is in the right direction. + +## Rejected ideas + +Why certain ideas that were brought while discussing this HIP were not +ultimately pursued. + +## Open issues + +[8661](https://github.com/helm/helm/issues/8661) + +## References + +A collection of URLs or materials used as references through the HIP. \ No newline at end of file From 39a043c85914f13c5a61e2ef205bf2643097eff5 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Thu, 12 Dec 2024 17:06:01 +0000 Subject: [PATCH 02/18] updates Signed-off-by: Austin Abro --- hips/hip-0999.md | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index dd011c47..c640dd67 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -9,13 +9,10 @@ status: "draft" ## Abstract -Currently the `--wait` flag on `helm install` and `helm upgrade` does not wait for all resources to be fully reconciled, and does not wait for custom resources (CRs) at all. By replacing the current wait logic with kstatus Helm will achieve *better* waits, while simplifying it's code and documentation. +Currently the `--wait` flag on `helm install` and `helm upgrade` does not wait for all resources to be fully reconciled, and does not wait for custom resources (CRs) at all. By replacing the current wait logic with kstatus, Helm will achieve more intuitive waits, while simplifying it's code and documentation. ## Motivation -Clearly explain why the existing design is inadequate to address the problem -that the HIP solves. - Certain workflows require CRDs to be ready before continuing to the next step. There is no way to tell Helm to wait for custom resources to be ready, so anyone that has this requirement must write their own logic to wait for their custom resources. Certain workflows may want to wait until their resources are fully reconciled before continuing to their next step. For example, Helm waits for all new pods in an upgraded deployment to be ready. However, Helm does not wait for the previous pods in that deployment to be removed. Ensuring only the new pods are available after waiting for an upgrade can come in handy. @@ -30,10 +27,20 @@ Describe why particular design decisions were made. Describe the syntax and semantics of any new feature. +From a CLI user's perspective there will be no changes in how waits are called, they will still use the `--wait` flag. + +Kstatus does not output any logs, Helm will supply it's own logs similar to the current logic. Helm with output a log message each second signalling that a resource is not ready. Helm will log one not ready resource at a time to not overwhelm users with output. The resource will be chosen alphabetically by `metadata.name`. + +A [dynamic rest mapper](https://github.com/kubernetes-sigs/controller-runtime/blob/aea2e32a936584b06ae6f7992f856fe7292b0297/pkg/client/apiutil/restmapper.go#L36) will be used, this way if a chart installs a custom resource definition, kstatus will be able to pkc up the custom resources during wait. + + + ## Backwards compatibility Describe potential impact and severity on pre-existing code. +Waiting for custom resources and for reconciliation to complete for every resource could lead to charts timing out that weren't previously. + ## Security implications No security implications @@ -44,15 +51,13 @@ Replace the [existing wait documentation](https://helm.sh/docs/intro/using_helm/ ## Reference implementation -Link to any existing implementation and details about its state, e.g. -proof-of-concept. +Once I have feedback that this HIP is in the right direction I will make a draft PR in Helm implementing this feature. -Here is an implementation of waiting for resources using kstatus in a project I maintain called Zarf. I will replace this section with a draft PR showing an example implementation in Helm once I have feedback that this HIP is in the right direction. +For now, see [healthchecks.go](https://github.com/zarf-dev/zarf/blob/main/src/internal/healthchecks/healthchecks.go) in a project I maintain called Zarf. This is how we implement kstatus. After each Helm install or upgrade in Zarf, we get every resource from the chart and send them into the `WaitForReady` function. ## Rejected ideas -Why certain ideas that were brought while discussing this HIP were not -ultimately pursued. +TBD ## Open issues @@ -60,4 +65,6 @@ ultimately pursued. ## References -A collection of URLs or materials used as references through the HIP. \ No newline at end of file +existing wait documentation - https://helm.sh/docs/intro/using_helm/ +kstatus documentation - https://github.com/kubernetes-sigs/cli-utils/blob/master/pkg/kstatus/README.md +Zarf kstatus implementation - \ No newline at end of file From c9dbf03318515040805bc3f9079df0033f37fd56 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Thu, 12 Dec 2024 17:18:11 +0000 Subject: [PATCH 03/18] grammar Signed-off-by: Austin Abro --- hips/hip-0999.md | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index c640dd67..70ffcac7 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -9,36 +9,32 @@ status: "draft" ## Abstract -Currently the `--wait` flag on `helm install` and `helm upgrade` does not wait for all resources to be fully reconciled, and does not wait for custom resources (CRs) at all. By replacing the current wait logic with kstatus, Helm will achieve more intuitive waits, while simplifying it's code and documentation. +Currently the `--wait` flag on `helm install` and `helm upgrade` does not wait for all resources to be fully reconciled, and does not wait for custom resources (CRs) at all. By replacing the wait logic with kstatus, Helm will achieve more intuitive waits, while simplifying its code and documentation. ## Motivation -Certain workflows require CRDs to be ready before continuing to the next step. There is no way to tell Helm to wait for custom resources to be ready, so anyone that has this requirement must write their own logic to wait for their custom resources. +Certain workflows require custom resources to be ready before continuing to the next step. There is no way to tell Helm to wait for custom resources to be ready, so anyone that has this requirement must write their own logic to wait for their custom resources. -Certain workflows may want to wait until their resources are fully reconciled before continuing to their next step. For example, Helm waits for all new pods in an upgraded deployment to be ready. However, Helm does not wait for the previous pods in that deployment to be removed. Ensuring only the new pods are available after waiting for an upgrade can come in handy. +Certain workflows may want to wait until their resources are fully reconciled before continuing to their next step. For example, Helm waits for all new pods in an upgraded deployment to be ready. However, Helm does not wait for the previous pods in that deployment to be removed. This ensures only the new pods are available after waiting for an upgrade can come in handy. By introducing kstatus we will lower user friction with the `--wait` flag. ## Rationale -Describe why particular design decisions were made. +Leveraging a existing status management tool maintained by the Kubernetes team will simplify the code and documentation that Helm needs to maintain and improve the functionality of `--wait`. ## Specification -Describe the syntax and semantics of any new feature. - From a CLI user's perspective there will be no changes in how waits are called, they will still use the `--wait` flag. -Kstatus does not output any logs, Helm will supply it's own logs similar to the current logic. Helm with output a log message each second signalling that a resource is not ready. Helm will log one not ready resource at a time to not overwhelm users with output. The resource will be chosen alphabetically by `metadata.name`. +Kstatus does not output any logs. Helm will output a log message each second signalling that a resource is not ready. Helm will log one not ready resource at a time to not overwhelm users with output. This behavior will look similar to the current `--wait` logging. The resource will be chosen alphabetically by `metadata.name`. -A [dynamic rest mapper](https://github.com/kubernetes-sigs/controller-runtime/blob/aea2e32a936584b06ae6f7992f856fe7292b0297/pkg/client/apiutil/restmapper.go#L36) will be used, this way if a chart installs a custom resource definition, kstatus will be able to pkc up the custom resources during wait. +A [dynamic rest mapper](https://github.com/kubernetes-sigs/controller-runtime/blob/aea2e32a936584b06ae6f7992f856fe7292b0297/pkg/client/apiutil/restmapper.go#L36) will be used, this way if a chart installs a custom resource definition, kstatus will be able to pick up the custom resources during wait. ## Backwards compatibility -Describe potential impact and severity on pre-existing code. - Waiting for custom resources and for reconciliation to complete for every resource could lead to charts timing out that weren't previously. ## Security implications @@ -47,7 +43,7 @@ No security implications ## How to teach this -Replace the [existing wait documentation](https://helm.sh/docs/intro/using_helm/) by explaining that we use kstatus and pointing to the [kstatus documentation](https://github.com/kubernetes-sigs/cli-utils/blob/master/pkg/kstatus/README.md). This comes with the added benefit of not needed to maintain Helm specific wait documentation. A separate team has already written extensive documentation on how the wait logic of kstatus works. +Replace the [existing wait documentation](https://helm.sh/docs/intro/using_helm/) by explaining that we use kstatus and pointing to the [kstatus documentation](https://github.com/kubernetes-sigs/cli-utils/blob/master/pkg/kstatus/README.md). This comes with the added benefit of not needing to maintain Helm specific wait documentation. A separate team has already written extensive documentation on how the wait logic of kstatus works. ## Reference implementation @@ -67,4 +63,4 @@ TBD existing wait documentation - https://helm.sh/docs/intro/using_helm/ kstatus documentation - https://github.com/kubernetes-sigs/cli-utils/blob/master/pkg/kstatus/README.md -Zarf kstatus implementation - \ No newline at end of file +Zarf kstatus implementation - https://github.com/zarf-dev/zarf/blob/main/src/internal/healthchecks/healthchecks.go \ No newline at end of file From 472f81a82a3e596fb72007af7ce128d4463aaf3e Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Thu, 12 Dec 2024 17:25:01 +0000 Subject: [PATCH 04/18] hip Signed-off-by: Austin Abro --- hips/hip-0999.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index 70ffcac7..9d879c16 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -9,19 +9,19 @@ status: "draft" ## Abstract -Currently the `--wait` flag on `helm install` and `helm upgrade` does not wait for all resources to be fully reconciled, and does not wait for custom resources (CRs) at all. By replacing the wait logic with kstatus, Helm will achieve more intuitive waits, while simplifying its code and documentation. +Currently the `--wait` flag on `helm install` and `helm upgrade` does not wait for all resources to be fully reconciled, and does not wait for custom resources (CRs) at all. By replacing the wait logic with [kstatus](https://github.com/kubernetes-sigs/cli-utils/blob/master/pkg/kstatus/README.md), Helm will achieve more intuitive waits, while simplifying its code and documentation. ## Motivation -Certain workflows require custom resources to be ready before continuing to the next step. There is no way to tell Helm to wait for custom resources to be ready, so anyone that has this requirement must write their own logic to wait for their custom resources. +Certain workflows require custom resources to be ready. There is no way to tell Helm to wait for custom resources to be ready, so anyone that has this requirement must write their own logic to wait for their custom resources. -Certain workflows may want to wait until their resources are fully reconciled before continuing to their next step. For example, Helm waits for all new pods in an upgraded deployment to be ready. However, Helm does not wait for the previous pods in that deployment to be removed. This ensures only the new pods are available after waiting for an upgrade can come in handy. +Certain workflows requires resources to be fully reconciled. For example, Helm waits for all new pods in an upgraded deployment to be ready. However, Helm does not wait for the previous pods in that deployment to be removed. By introducing kstatus we will lower user friction with the `--wait` flag. ## Rationale -Leveraging a existing status management tool maintained by the Kubernetes team will simplify the code and documentation that Helm needs to maintain and improve the functionality of `--wait`. +Leveraging a existing status management library maintained by the Kubernetes team will simplify the code and documentation that Helm needs to maintain and improve the functionality of `--wait`. ## Specification @@ -43,13 +43,13 @@ No security implications ## How to teach this -Replace the [existing wait documentation](https://helm.sh/docs/intro/using_helm/) by explaining that we use kstatus and pointing to the [kstatus documentation](https://github.com/kubernetes-sigs/cli-utils/blob/master/pkg/kstatus/README.md). This comes with the added benefit of not needing to maintain Helm specific wait documentation. A separate team has already written extensive documentation on how the wait logic of kstatus works. +Replace the [existing wait documentation](https://helm.sh/docs/intro/using_helm/) by explaining that we use kstatus and pointing to the [kstatus documentation](https://github.com/kubernetes-sigs/cli-utils/blob/master/pkg/kstatus/README.md). This comes with the added benefit of not needing to maintain Helm specific wait documentation. ## Reference implementation Once I have feedback that this HIP is in the right direction I will make a draft PR in Helm implementing this feature. -For now, see [healthchecks.go](https://github.com/zarf-dev/zarf/blob/main/src/internal/healthchecks/healthchecks.go) in a project I maintain called Zarf. This is how we implement kstatus. After each Helm install or upgrade in Zarf, we get every resource from the chart and send them into the `WaitForReady` function. +For now, see [healthchecks.go](https://github.com/zarf-dev/zarf/blob/main/src/internal/healthchecks/healthchecks.go) in a project I help maintain called Zarf. After each Helm install or upgrade in Zarf, we get every resource from the chart and send them into the `WaitForReady` function. ## Rejected ideas From dcf6c8b1fc0c4144f8c2116cd99d3ffd25f3eaca Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Sat, 14 Dec 2024 18:21:45 +0000 Subject: [PATCH 05/18] updates Signed-off-by: Austin Abro --- hips/hip-0999.md | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index 9d879c16..e86260f8 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -13,9 +13,9 @@ Currently the `--wait` flag on `helm install` and `helm upgrade` does not wait f ## Motivation -Certain workflows require custom resources to be ready. There is no way to tell Helm to wait for custom resources to be ready, so anyone that has this requirement must write their own logic to wait for their custom resources. +Certain workflows require custom resources to be ready. There is no way to tell Helm to wait for custom resources to be ready, so anyone with this requirement must write their own logic to wait for their custom resources. Kstatus solves this by waiting for custom resources to have [the ready condition](https://github.com/kubernetes-sigs/cli-utils/blob/master/pkg/kstatus/README.md#the-ready-condition) set to true. -Certain workflows requires resources to be fully reconciled. For example, Helm waits for all new pods in an upgraded deployment to be ready. However, Helm does not wait for the previous pods in that deployment to be removed. +Certain workflows requires resources to be fully reconciled, which does happen in the current `--wait` logic. For example, Helm waits for all new pods in an upgraded deployment to be ready. However, Helm does not wait for the previous pods in that deployment to be removed. Since kstatus waits for all resources to be reconciled, the wait will not finish for a deployment until all of its new pods are ready and all of its old pods have been deleted. By introducing kstatus we will lower user friction with the `--wait` flag. @@ -27,16 +27,35 @@ Leveraging a existing status management library maintained by the Kubernetes tea From a CLI user's perspective there will be no changes in how waits are called, they will still use the `--wait` flag. -Kstatus does not output any logs. Helm will output a log message each second signalling that a resource is not ready. Helm will log one not ready resource at a time to not overwhelm users with output. This behavior will look similar to the current `--wait` logging. The resource will be chosen alphabetically by `metadata.name`. +Kstatus does not output any logs. Helm will output a log message each second signalling that a resource is not ready. Helm will log one of the resources it's waiting on at a time as to not overwhelm users with output. This behavior will look similar to the current `--wait` logging. + + A [dynamic rest mapper](https://github.com/kubernetes-sigs/controller-runtime/blob/aea2e32a936584b06ae6f7992f856fe7292b0297/pkg/client/apiutil/restmapper.go#L36) will be used, this way if a chart installs a custom resource definition, kstatus will be able to pick up the custom resources during wait. - +Kstatus will not be exposed by the Helm SDK, instead an adapter/interface will be created. If kstatus is ever deprecated, unmaintained, or replaced there should be no impact to the public API of the Helm SDK. + + ## Backwards compatibility Waiting for custom resources and for reconciliation to complete for every resource could lead to charts timing out that weren't previously. +Kstatus does not require any general RBAC additions, such as events. This can be tested by following instructions in this repo https://github.com/AustinAbro321/kstatus-rbac-test. + +Kstatus does require some RBAC permissions for child resources. For instance, checking if a deployment is ready requires "list" permissions for the replicaset. Currently, Helm's logic also necessitates RBAC permissions to "list" child replicasets, so this requirement is not new. However, there may be cases where the RBAC requirements for child resources differ between Kstatus and Helm, as an evaluation has not been conducted. + +```yaml +rules: + - apiGroups: ["apps"] + resources: ["deployments"] + verbs: ["list", "watch"] + - apiGroups: ["apps"] + resources: ["replicasets"] + verbs: ["list"] +``` + + ## Security implications No security implications From c049e80c98ad6c83e9363ad337b061836bf4a704 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Mon, 6 Jan 2025 15:25:00 +0000 Subject: [PATCH 06/18] updates to new architecture Signed-off-by: Austin Abro --- hips/hip-0999.md | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index e86260f8..c5528084 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -33,9 +33,30 @@ Kstatus does not output any logs. Helm will output a log message each second sig A [dynamic rest mapper](https://github.com/kubernetes-sigs/controller-runtime/blob/aea2e32a936584b06ae6f7992f856fe7292b0297/pkg/client/apiutil/restmapper.go#L36) will be used, this way if a chart installs a custom resource definition, kstatus will be able to pick up the custom resources during wait. -Kstatus will not be exposed by the Helm SDK, instead an adapter/interface will be created. If kstatus is ever deprecated, unmaintained, or replaced there should be no impact to the public API of the Helm SDK. +Any functions involving waits will be separated from the `kube.Interface` interface into the `kube.Waiter` interface. The client will embed the `Waiter` interface to allow calls to look like `client.Wait()` instead of `client.waiter.Wait()`. `kube.New()` will accept a wait strategy to decide the wait implementation. Options will be either `HelmWaiter` or `statusWaiter`. `HelmWaiter` is the legacy implementation. The `statusWaiter` will not be public so that is kstatus is ever deprecated or replaced a new implementation can be used without changing the public SDK. + +The new client will look like below: +```go +type Client struct { + Factory Factory + Log func(string, ...interface{}) + Namespace string + kubeClient *kubernetes.Clientset + Waiter +} +type WaitStrategy int +const ( + StatusWaiter WaitStrategy = iota + LegacyWaiter +) +func New(getter genericclioptions.RESTClientGetter, ws WaitStrategy) (*Client, error) +``` + +The Helm CLI will always use the `statusWaiter` implementation, if this is found to be insufficient during Helm 4 testing a new cli flag `wait-strategy` will be introduced with options `status` and `legacy` to allow usage of the `HelmWaiter`. If the `statusWaiter` is found to be sufficient the `HelmWaiter` will be deleted from the public SDK before Helm 4 is released. + +`WaitAndGetCompletedPodPhase` is an exported function that is not called anywhere within the Helm repository. It will be removed. - +The current Helm wait logic does not wait for paused deployments to be ready. This is because if `helm upgrade --wait` is run on a chart with paused deployments they will never be ready, see [#5789](https://github.com/helm/helm/pull/5789). ## Backwards compatibility From 0c7da3fc9373b177be9a1283d412cf5fbfd8174b Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Mon, 6 Jan 2025 15:30:51 +0000 Subject: [PATCH 07/18] updates to new architecture Signed-off-by: Austin Abro --- hips/hip-0999.md | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index c5528084..7d123323 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -33,9 +33,9 @@ Kstatus does not output any logs. Helm will output a log message each second sig A [dynamic rest mapper](https://github.com/kubernetes-sigs/controller-runtime/blob/aea2e32a936584b06ae6f7992f856fe7292b0297/pkg/client/apiutil/restmapper.go#L36) will be used, this way if a chart installs a custom resource definition, kstatus will be able to pick up the custom resources during wait. -Any functions involving waits will be separated from the `kube.Interface` interface into the `kube.Waiter` interface. The client will embed the `Waiter` interface to allow calls to look like `client.Wait()` instead of `client.waiter.Wait()`. `kube.New()` will accept a wait strategy to decide the wait implementation. Options will be either `HelmWaiter` or `statusWaiter`. `HelmWaiter` is the legacy implementation. The `statusWaiter` will not be public so that is kstatus is ever deprecated or replaced a new implementation can be used without changing the public SDK. +Any functions involving waits will be separated from the `kube.Interface` interface into the `kube.Waiter` interface. `kube.Waiter` will be embedded into `kube.Interface`. The client struct will embed the `Waiter` interface to allow calls to look like `client.Wait()` instead of `client.Waiter.Wait()`. `kube.New()` will accept a wait strategy to decide the wait implementation. Options will be either `HelmWaiter` or `statusWaiter`. `HelmWaiter` is the legacy implementation. The `statusWaiter` will not be public so that is kstatus is ever deprecated or replaced a new implementation can be used without changing the public SDK. -The new client will look like below: +The new client will look like: ```go type Client struct { Factory Factory @@ -52,10 +52,21 @@ const ( func New(getter genericclioptions.RESTClientGetter, ws WaitStrategy) (*Client, error) ``` -The Helm CLI will always use the `statusWaiter` implementation, if this is found to be insufficient during Helm 4 testing a new cli flag `wait-strategy` will be introduced with options `status` and `legacy` to allow usage of the `HelmWaiter`. If the `statusWaiter` is found to be sufficient the `HelmWaiter` will be deleted from the public SDK before Helm 4 is released. +The waiter interface will look like: +```go +type Waiter interface { + Wait(resources ResourceList, timeout time.Duration) error + WaitWithJobs(resources ResourceList, timeout time.Duration) error + WaitForDelete(resources ResourceList, timeout time.Duration) error +} +``` `WaitAndGetCompletedPodPhase` is an exported function that is not called anywhere within the Helm repository. It will be removed. +`WatchUntilReady` is used only for hooks. It has custom wait logic different from the Helm 3 general logic. Ideally, this could be replaced with a regular `Wait()` call. If there is any historical context as to why this logic is the way it is, please share. + +The Helm CLI will always use the `statusWaiter` implementation, if this is found to be insufficient during Helm 4 testing a new cli flag `wait-strategy` will be introduced with options `status` and `legacy` to allow usage of the `HelmWaiter`. If the `statusWaiter` is found to be sufficient the `HelmWaiter` will be deleted from the public SDK before Helm 4 is released. + The current Helm wait logic does not wait for paused deployments to be ready. This is because if `helm upgrade --wait` is run on a chart with paused deployments they will never be ready, see [#5789](https://github.com/helm/helm/pull/5789). ## Backwards compatibility From 467dde60506b80aca5726649f3ac0c59bf58eab3 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Wed, 8 Jan 2025 13:50:38 +0000 Subject: [PATCH 08/18] mention watch and polling Signed-off-by: Austin Abro --- hips/hip-0999.md | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index 7d123323..45f04e43 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -29,8 +29,6 @@ From a CLI user's perspective there will be no changes in how waits are called, Kstatus does not output any logs. Helm will output a log message each second signalling that a resource is not ready. Helm will log one of the resources it's waiting on at a time as to not overwhelm users with output. This behavior will look similar to the current `--wait` logging. - - A [dynamic rest mapper](https://github.com/kubernetes-sigs/controller-runtime/blob/aea2e32a936584b06ae6f7992f856fe7292b0297/pkg/client/apiutil/restmapper.go#L36) will be used, this way if a chart installs a custom resource definition, kstatus will be able to pick up the custom resources during wait. Any functions involving waits will be separated from the `kube.Interface` interface into the `kube.Waiter` interface. `kube.Waiter` will be embedded into `kube.Interface`. The client struct will embed the `Waiter` interface to allow calls to look like `client.Wait()` instead of `client.Waiter.Wait()`. `kube.New()` will accept a wait strategy to decide the wait implementation. Options will be either `HelmWaiter` or `statusWaiter`. `HelmWaiter` is the legacy implementation. The `statusWaiter` will not be public so that is kstatus is ever deprecated or replaced a new implementation can be used without changing the public SDK. @@ -73,15 +71,14 @@ The current Helm wait logic does not wait for paused deployments to be ready. Th Waiting for custom resources and for reconciliation to complete for every resource could lead to charts timing out that weren't previously. -Kstatus does not require any general RBAC additions, such as events. This can be tested by following instructions in this repo https://github.com/AustinAbro321/kstatus-rbac-test. +The kstatus status watcher requires the "list" and "watch" RBAC permissions to watch a resource. The kstatus status poller and current Helm implementation only require "list" permissions for the resources they're watching. Kstatus and Helm require "list" permissions for some child resources. For instance, checking if a deployment is ready requires "list" permissions for the replicaset. There may be cases where the RBAC requirements for child resources differ between Kstatus and Helm, as an evaluation has not been conducted. -Kstatus does require some RBAC permissions for child resources. For instance, checking if a deployment is ready requires "list" permissions for the replicaset. Currently, Helm's logic also necessitates RBAC permissions to "list" child replicasets, so this requirement is not new. However, there may be cases where the RBAC requirements for child resources differ between Kstatus and Helm, as an evaluation has not been conducted. - +Below is the minimal set needed to watch a deployment with the status watcher. This can be verified by following instructions in this repo: https://github.com/AustinAbro321/kstatus-rbac-test. ```yaml rules: - apiGroups: ["apps"] resources: ["deployments"] - verbs: ["list", "watch"] + verbs: ["list", "watch"] # The status poller only requires "list" here - apiGroups: ["apps"] resources: ["replicasets"] verbs: ["list"] @@ -90,7 +87,7 @@ rules: ## Security implications -No security implications +Users will now need "watch" permissions on resources in their chart if `--wait` is used, assuming the status watcher is used over the status poller. ## How to teach this @@ -98,9 +95,7 @@ Replace the [existing wait documentation](https://helm.sh/docs/intro/using_helm/ ## Reference implementation -Once I have feedback that this HIP is in the right direction I will make a draft PR in Helm implementing this feature. - -For now, see [healthchecks.go](https://github.com/zarf-dev/zarf/blob/main/src/internal/healthchecks/healthchecks.go) in a project I help maintain called Zarf. After each Helm install or upgrade in Zarf, we get every resource from the chart and send them into the `WaitForReady` function. +seen here - https://github.com/helm/helm/pull/13604 ## Rejected ideas From bb2b1902c50f203d80cbdcbd667a2cce4ba173bb Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Wed, 8 Jan 2025 15:56:09 +0000 Subject: [PATCH 09/18] mention poller vs watcher Signed-off-by: Austin Abro --- hips/hip-0999.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index 45f04e43..1151e6bd 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -29,7 +29,7 @@ From a CLI user's perspective there will be no changes in how waits are called, Kstatus does not output any logs. Helm will output a log message each second signalling that a resource is not ready. Helm will log one of the resources it's waiting on at a time as to not overwhelm users with output. This behavior will look similar to the current `--wait` logging. -A [dynamic rest mapper](https://github.com/kubernetes-sigs/controller-runtime/blob/aea2e32a936584b06ae6f7992f856fe7292b0297/pkg/client/apiutil/restmapper.go#L36) will be used, this way if a chart installs a custom resource definition, kstatus will be able to pick up the custom resources during wait. +Kstatus can be used with either a poller or a watcher. The poller runs on a specified interval and only requires "list" RBAC permissions for polled resources. The watcher reacts to [watch events](https://github.com/kubernetes/kubernetes/blob/90a45563ae1bab5868ee888432fec9aac2f7f8b1/staging/src/k8s.io/apimachinery/pkg/watch/watch.go#L55-L61) and requires "list" and "watch" RBAC permission. This proposal uses the watcher as it responds slightly faster when all resources are ready, however, if the additional RBAC permissions are deemed to cause potential issues, we can instead switch to the poller. Any functions involving waits will be separated from the `kube.Interface` interface into the `kube.Waiter` interface. `kube.Waiter` will be embedded into `kube.Interface`. The client struct will embed the `Waiter` interface to allow calls to look like `client.Wait()` instead of `client.Waiter.Wait()`. `kube.New()` will accept a wait strategy to decide the wait implementation. Options will be either `HelmWaiter` or `statusWaiter`. `HelmWaiter` is the legacy implementation. The `statusWaiter` will not be public so that is kstatus is ever deprecated or replaced a new implementation can be used without changing the public SDK. From 5275514523b0ca36ff767c274c3e9ad352d2e810 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Wed, 22 Jan 2025 17:47:48 +0000 Subject: [PATCH 10/18] update why around watch Signed-off-by: Austin Abro --- hips/hip-0999.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index 1151e6bd..38e0deba 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -29,7 +29,7 @@ From a CLI user's perspective there will be no changes in how waits are called, Kstatus does not output any logs. Helm will output a log message each second signalling that a resource is not ready. Helm will log one of the resources it's waiting on at a time as to not overwhelm users with output. This behavior will look similar to the current `--wait` logging. -Kstatus can be used with either a poller or a watcher. The poller runs on a specified interval and only requires "list" RBAC permissions for polled resources. The watcher reacts to [watch events](https://github.com/kubernetes/kubernetes/blob/90a45563ae1bab5868ee888432fec9aac2f7f8b1/staging/src/k8s.io/apimachinery/pkg/watch/watch.go#L55-L61) and requires "list" and "watch" RBAC permission. This proposal uses the watcher as it responds slightly faster when all resources are ready, however, if the additional RBAC permissions are deemed to cause potential issues, we can instead switch to the poller. +Kstatus can be used with either a poller or a watcher. The poller runs on a specified interval and only requires "list" RBAC permissions for polled resources. The watcher reacts to [watch events](https://github.com/kubernetes/kubernetes/blob/90a45563ae1bab5868ee888432fec9aac2f7f8b1/staging/src/k8s.io/apimachinery/pkg/watch/watch.go#L55-L61) and requires "list" and "watch" RBAC permissions. This proposal uses the watcher as it responds slightly faster when all resources are ready, and it is very likely that users applying or deleting resources will have "watch" permissions on their resources. However, if the additional RBAC permissions are deemed to cause potential issues the poller can be used instead. Any functions involving waits will be separated from the `kube.Interface` interface into the `kube.Waiter` interface. `kube.Waiter` will be embedded into `kube.Interface`. The client struct will embed the `Waiter` interface to allow calls to look like `client.Wait()` instead of `client.Waiter.Wait()`. `kube.New()` will accept a wait strategy to decide the wait implementation. Options will be either `HelmWaiter` or `statusWaiter`. `HelmWaiter` is the legacy implementation. The `statusWaiter` will not be public so that is kstatus is ever deprecated or replaced a new implementation can be used without changing the public SDK. From 1787ccf26369de722aaecd4ccfcdc5bc897908b4 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Mon, 10 Feb 2025 15:44:15 +0000 Subject: [PATCH 11/18] remove poller Signed-off-by: Austin Abro --- hips/hip-0999.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index 38e0deba..36fc948b 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -27,9 +27,9 @@ Leveraging a existing status management library maintained by the Kubernetes tea From a CLI user's perspective there will be no changes in how waits are called, they will still use the `--wait` flag. -Kstatus does not output any logs. Helm will output a log message each second signalling that a resource is not ready. Helm will log one of the resources it's waiting on at a time as to not overwhelm users with output. This behavior will look similar to the current `--wait` logging. +Kstatus does not output any logs. Helm will output a custom log message of a resources it's waiting on. Helm will log one resource at a time as to not overwhelm users with output. -Kstatus can be used with either a poller or a watcher. The poller runs on a specified interval and only requires "list" RBAC permissions for polled resources. The watcher reacts to [watch events](https://github.com/kubernetes/kubernetes/blob/90a45563ae1bab5868ee888432fec9aac2f7f8b1/staging/src/k8s.io/apimachinery/pkg/watch/watch.go#L55-L61) and requires "list" and "watch" RBAC permissions. This proposal uses the watcher as it responds slightly faster when all resources are ready, and it is very likely that users applying or deleting resources will have "watch" permissions on their resources. However, if the additional RBAC permissions are deemed to cause potential issues the poller can be used instead. +Kstatus can be used with either a poller or a watcher. The poller runs on a specified interval and only requires "list" RBAC permissions for polled resources. The watcher reacts to [watch events](https://github.com/kubernetes/kubernetes/blob/90a45563ae1bab5868ee888432fec9aac2f7f8b1/staging/src/k8s.io/apimachinery/pkg/watch/watch.go#L55-L61) and requires "list" and "watch" RBAC permissions. This proposal uses the watcher as it responds slightly faster when all resources are ready, and it is very likely that users applying or deleting resources will have "watch" permissions on their resources. Any functions involving waits will be separated from the `kube.Interface` interface into the `kube.Waiter` interface. `kube.Waiter` will be embedded into `kube.Interface`. The client struct will embed the `Waiter` interface to allow calls to look like `client.Wait()` instead of `client.Waiter.Wait()`. `kube.New()` will accept a wait strategy to decide the wait implementation. Options will be either `HelmWaiter` or `statusWaiter`. `HelmWaiter` is the legacy implementation. The `statusWaiter` will not be public so that is kstatus is ever deprecated or replaced a new implementation can be used without changing the public SDK. @@ -69,16 +69,20 @@ The current Helm wait logic does not wait for paused deployments to be ready. Th ## Backwards compatibility +To avoid any issues with backwards compatibility the existing wait flag will be extended to accept the following fields `--wait=true|false|none|watcher|legacy`. (true & false are placeholders for kstatus and none respectively, to be compatible with existing users who might have --wait=true, if we decide this is warranted for Helm 4. else we just error those users) + Waiting for custom resources and for reconciliation to complete for every resource could lead to charts timing out that weren't previously. -The kstatus status watcher requires the "list" and "watch" RBAC permissions to watch a resource. The kstatus status poller and current Helm implementation only require "list" permissions for the resources they're watching. Kstatus and Helm require "list" permissions for some child resources. For instance, checking if a deployment is ready requires "list" permissions for the replicaset. There may be cases where the RBAC requirements for child resources differ between Kstatus and Helm, as an evaluation has not been conducted. +The kstatus status watcher requires the "list" and "watch" RBAC permissions to watch a resource. The current Helm implementation only require "list" permissions for the resources they're watching. Kstatus and Helm require "list" permissions for some child resources. For instance, checking if a deployment is ready requires "list" permissions for the replicaset. There may be cases where the RBAC requirements for child resources differ between Kstatus and Helm, as an evaluation has not been conducted. + +Kstatus also watches more resources than Helm does. A user will need "list" and "watch" permissions to every resource that they are deploying. Currently, Helm only checks readiness of certain resources. See the [IsReady function](https://github.com/helm/helm/blob/0d66425d9a745d8a289b1a5ebb6ccc744436da95/pkg/kube/ready.go#L92) for details Below is the minimal set needed to watch a deployment with the status watcher. This can be verified by following instructions in this repo: https://github.com/AustinAbro321/kstatus-rbac-test. ```yaml rules: - apiGroups: ["apps"] resources: ["deployments"] - verbs: ["list", "watch"] # The status poller only requires "list" here + verbs: ["list", "watch"] - apiGroups: ["apps"] resources: ["replicasets"] verbs: ["list"] @@ -87,7 +91,7 @@ rules: ## Security implications -Users will now need "watch" permissions on resources in their chart if `--wait` is used, assuming the status watcher is used over the status poller. +Users will now need "watch" permissions on resources in their chart if the `--wait` flag is used. ## How to teach this From 77fcc9fbe7b0c418281efeb91287f881ef0ef9ba Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Mon, 10 Feb 2025 16:08:42 +0000 Subject: [PATCH 12/18] changes based on comments Signed-off-by: Austin Abro --- hips/hip-0999.md | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index 36fc948b..7f242fa2 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -56,16 +56,23 @@ type Waiter interface { Wait(resources ResourceList, timeout time.Duration) error WaitWithJobs(resources ResourceList, timeout time.Duration) error WaitForDelete(resources ResourceList, timeout time.Duration) error + WatchUntilReady(resources ResourceList, timeout time.Duration) error } ``` -`WaitAndGetCompletedPodPhase` is an exported function that is not called anywhere within the Helm repository. It will be removed. +Below is a summary of the kstatus implementation of each function: -`WatchUntilReady` is used only for hooks. It has custom wait logic different from the Helm 3 general logic. Ideally, this could be replaced with a regular `Wait()` call. If there is any historical context as to why this logic is the way it is, please share. +`Wait` will wait for all resources to be ready. This will include jobs, but this function not wait for jobs to be complete. + +`WaitWithJobs` will wait for all resources to be ready and all jobs to be complete + +`WatchUntilReady` only waits for Pods and Jobs to complete. It is used for Helm hooks. This logic will stay the same The Helm CLI will always use the `statusWaiter` implementation, if this is found to be insufficient during Helm 4 testing a new cli flag `wait-strategy` will be introduced with options `status` and `legacy` to allow usage of the `HelmWaiter`. If the `statusWaiter` is found to be sufficient the `HelmWaiter` will be deleted from the public SDK before Helm 4 is released. -The current Helm wait logic does not wait for paused deployments to be ready. This is because if `helm upgrade --wait` is run on a chart with paused deployments they will never be ready, see [#5789](https://github.com/helm/helm/pull/5789). +Calls to `Wait` and `WaitWithJobs` will not wait for paused deployments. This is consistent with the current logic. This is done because otherwise `helm upgrade --wait` on a paused deployment will never be ready, see [#5789](https://github.com/helm/helm/pull/5789). + +The Helm CLI will continue to use the existing `--wait` flag. It will have the options `--wait=true|false|none|watcher|legacy`. Watcher will be the new implementation proposed in this HIP. true & false are placeholders for "watcher" and none respectively, to be compatible with existing users who might have `--wait=true`. ## Backwards compatibility @@ -91,7 +98,7 @@ rules: ## Security implications -Users will now need "watch" permissions on resources in their chart if the `--wait` flag is used. +Users will now need "watch" permissions on resources in their chart if the `--wait` flag is used. They will also need "list" and "watch" permissions for all resources they are deploying rather than just the resources that Helm currently waits for. ## How to teach this From cf055501c6c0ed2748a222b21ceaf2f207cb212e Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Mon, 10 Feb 2025 16:10:24 +0000 Subject: [PATCH 13/18] clearer line Signed-off-by: Austin Abro --- hips/hip-0999.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index 7f242fa2..93128af4 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -15,7 +15,7 @@ Currently the `--wait` flag on `helm install` and `helm upgrade` does not wait f Certain workflows require custom resources to be ready. There is no way to tell Helm to wait for custom resources to be ready, so anyone with this requirement must write their own logic to wait for their custom resources. Kstatus solves this by waiting for custom resources to have [the ready condition](https://github.com/kubernetes-sigs/cli-utils/blob/master/pkg/kstatus/README.md#the-ready-condition) set to true. -Certain workflows requires resources to be fully reconciled, which does happen in the current `--wait` logic. For example, Helm waits for all new pods in an upgraded deployment to be ready. However, Helm does not wait for the previous pods in that deployment to be removed. Since kstatus waits for all resources to be reconciled, the wait will not finish for a deployment until all of its new pods are ready and all of its old pods have been deleted. +Certain workflows requires resources to be fully reconciled, which does happen in the current `--wait` logic. For example, Helm waits for all new pods in an upgraded deployment to be ready. However, Helm does not wait for the previous pods in that deployment to be removed. Therefore, Helm will finish waiting even while old pods are still active. Since kstatus instead waits for all resources to be reconciled, the wait will not finish for a deployment until all of its new pods are ready and all of its old pods have been deleted. By introducing kstatus we will lower user friction with the `--wait` flag. From 6eb9d2ac14d86d003edec5a43e83772d1bc2b79d Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Mon, 10 Feb 2025 16:10:39 +0000 Subject: [PATCH 14/18] move line Signed-off-by: Austin Abro --- hips/hip-0999.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index 93128af4..73f078f6 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -13,12 +13,12 @@ Currently the `--wait` flag on `helm install` and `helm upgrade` does not wait f ## Motivation +By introducing kstatus we will lower user friction with the `--wait` flag. + Certain workflows require custom resources to be ready. There is no way to tell Helm to wait for custom resources to be ready, so anyone with this requirement must write their own logic to wait for their custom resources. Kstatus solves this by waiting for custom resources to have [the ready condition](https://github.com/kubernetes-sigs/cli-utils/blob/master/pkg/kstatus/README.md#the-ready-condition) set to true. Certain workflows requires resources to be fully reconciled, which does happen in the current `--wait` logic. For example, Helm waits for all new pods in an upgraded deployment to be ready. However, Helm does not wait for the previous pods in that deployment to be removed. Therefore, Helm will finish waiting even while old pods are still active. Since kstatus instead waits for all resources to be reconciled, the wait will not finish for a deployment until all of its new pods are ready and all of its old pods have been deleted. -By introducing kstatus we will lower user friction with the `--wait` flag. - ## Rationale Leveraging a existing status management library maintained by the Kubernetes team will simplify the code and documentation that Helm needs to maintain and improve the functionality of `--wait`. From 9649edd7ced3f91da605caa621e96be0ce4318eb Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Mon, 10 Feb 2025 16:19:00 +0000 Subject: [PATCH 15/18] update Signed-off-by: Austin Abro --- hips/hip-0999.md | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index 73f078f6..2c31648d 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -25,13 +25,13 @@ Leveraging a existing status management library maintained by the Kubernetes tea ## Specification -From a CLI user's perspective there will be no changes in how waits are called, they will still use the `--wait` flag. +The Helm CLI will continue to use the existing `--wait` flag. It will have the options `--wait=true|false|none|watcher|legacy`. Watcher will be the new implementation proposed in this HIP. `true` & `false` are placeholders for `watcher` and `none` respectively, to be compatible with existing users who might have `--wait=true`. -Kstatus does not output any logs. Helm will output a custom log message of a resources it's waiting on. Helm will log one resource at a time as to not overwhelm users with output. +Kstatus does not natively output any logs. During wait, Helm will output a log message of one resources that's not ready. Only one resource is logged at a time so Helm doesn't overwhelm users with output. Kstatus can be used with either a poller or a watcher. The poller runs on a specified interval and only requires "list" RBAC permissions for polled resources. The watcher reacts to [watch events](https://github.com/kubernetes/kubernetes/blob/90a45563ae1bab5868ee888432fec9aac2f7f8b1/staging/src/k8s.io/apimachinery/pkg/watch/watch.go#L55-L61) and requires "list" and "watch" RBAC permissions. This proposal uses the watcher as it responds slightly faster when all resources are ready, and it is very likely that users applying or deleting resources will have "watch" permissions on their resources. -Any functions involving waits will be separated from the `kube.Interface` interface into the `kube.Waiter` interface. `kube.Waiter` will be embedded into `kube.Interface`. The client struct will embed the `Waiter` interface to allow calls to look like `client.Wait()` instead of `client.Waiter.Wait()`. `kube.New()` will accept a wait strategy to decide the wait implementation. Options will be either `HelmWaiter` or `statusWaiter`. `HelmWaiter` is the legacy implementation. The `statusWaiter` will not be public so that is kstatus is ever deprecated or replaced a new implementation can be used without changing the public SDK. +Any functions involving waits will be separated from the `kube.Interface` interface into the `kube.Waiter` interface. `kube.Waiter` will be embedded into `kube.Interface`. The client struct will embed the `Waiter` interface to allow calls to look like `client.Wait()` instead of `client.Waiter.Wait()`. `kube.New()` will accept a wait strategy to decide the wait implementation. There will be two implementation in the repo, `HelmWaiter` and `statusWaiter`. `HelmWaiter` is the legacy implementation. The `statusWaiter` will not be public so that if kstatus is ever deprecated or replaced a new implementation can be used without changing the public SDK. The new client will look like: ```go @@ -60,25 +60,17 @@ type Waiter interface { } ``` -Below is a summary of the kstatus implementation of each function: - `Wait` will wait for all resources to be ready. This will include jobs, but this function not wait for jobs to be complete. -`WaitWithJobs` will wait for all resources to be ready and all jobs to be complete - -`WatchUntilReady` only waits for Pods and Jobs to complete. It is used for Helm hooks. This logic will stay the same +`WaitWithJobs` will wait for all resources to be ready and all jobs to be complete. -The Helm CLI will always use the `statusWaiter` implementation, if this is found to be insufficient during Helm 4 testing a new cli flag `wait-strategy` will be introduced with options `status` and `legacy` to allow usage of the `HelmWaiter`. If the `statusWaiter` is found to be sufficient the `HelmWaiter` will be deleted from the public SDK before Helm 4 is released. +`WatchUntilReady` only waits for Pods and Jobs to complete. It is used for Helm hooks. This logic will stay the same. Calls to `Wait` and `WaitWithJobs` will not wait for paused deployments. This is consistent with the current logic. This is done because otherwise `helm upgrade --wait` on a paused deployment will never be ready, see [#5789](https://github.com/helm/helm/pull/5789). -The Helm CLI will continue to use the existing `--wait` flag. It will have the options `--wait=true|false|none|watcher|legacy`. Watcher will be the new implementation proposed in this HIP. true & false are placeholders for "watcher" and none respectively, to be compatible with existing users who might have `--wait=true`. - ## Backwards compatibility -To avoid any issues with backwards compatibility the existing wait flag will be extended to accept the following fields `--wait=true|false|none|watcher|legacy`. (true & false are placeholders for kstatus and none respectively, to be compatible with existing users who might have --wait=true, if we decide this is warranted for Helm 4. else we just error those users) - -Waiting for custom resources and for reconciliation to complete for every resource could lead to charts timing out that weren't previously. +Waiting for custom resources and other previously not waited for resources could lead to charts timing out when using the new logic. The kstatus status watcher requires the "list" and "watch" RBAC permissions to watch a resource. The current Helm implementation only require "list" permissions for the resources they're watching. Kstatus and Helm require "list" permissions for some child resources. For instance, checking if a deployment is ready requires "list" permissions for the replicaset. There may be cases where the RBAC requirements for child resources differ between Kstatus and Helm, as an evaluation has not been conducted. From 97539182377ffecf352f08fca2b1e0e2c6a8c971 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Wed, 12 Feb 2025 13:57:06 +0000 Subject: [PATCH 16/18] wait & wait strategy Signed-off-by: Austin Abro --- hips/hip-0999.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index 2c31648d..36754a28 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -25,9 +25,7 @@ Leveraging a existing status management library maintained by the Kubernetes tea ## Specification -The Helm CLI will continue to use the existing `--wait` flag. It will have the options `--wait=true|false|none|watcher|legacy`. Watcher will be the new implementation proposed in this HIP. `true` & `false` are placeholders for `watcher` and `none` respectively, to be compatible with existing users who might have `--wait=true`. - -Kstatus does not natively output any logs. During wait, Helm will output a log message of one resources that's not ready. Only one resource is logged at a time so Helm doesn't overwhelm users with output. +The Helm CLI will continue to use the existing `--wait` flag. A new flag `--wait-strategy` will be introduced. It will have the options `Watcher` and `legacy`. `--wait-strategy` will default to watcher, which will use kstatus. Kstatus can be used with either a poller or a watcher. The poller runs on a specified interval and only requires "list" RBAC permissions for polled resources. The watcher reacts to [watch events](https://github.com/kubernetes/kubernetes/blob/90a45563ae1bab5868ee888432fec9aac2f7f8b1/staging/src/k8s.io/apimachinery/pkg/watch/watch.go#L55-L61) and requires "list" and "watch" RBAC permissions. This proposal uses the watcher as it responds slightly faster when all resources are ready, and it is very likely that users applying or deleting resources will have "watch" permissions on their resources. @@ -68,6 +66,8 @@ type Waiter interface { Calls to `Wait` and `WaitWithJobs` will not wait for paused deployments. This is consistent with the current logic. This is done because otherwise `helm upgrade --wait` on a paused deployment will never be ready, see [#5789](https://github.com/helm/helm/pull/5789). +Kstatus does not natively output any logs. After each event, Helm will output a log message of one resources that's not ready. Only one resource is logged at a time so Helm doesn't overwhelm users with output. Note that the logs are only sent when called as a library, the CLI uses a noop logger for Kube operations. + ## Backwards compatibility Waiting for custom resources and other previously not waited for resources could lead to charts timing out when using the new logic. From 50cd1426b78f0faf0da93133b5581dd9c41554bb Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Fri, 14 Feb 2025 19:51:49 +0000 Subject: [PATCH 17/18] update wait flag Signed-off-by: Austin Abro --- hips/hip-0999.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index 36754a28..218b1c89 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -25,7 +25,7 @@ Leveraging a existing status management library maintained by the Kubernetes tea ## Specification -The Helm CLI will continue to use the existing `--wait` flag. A new flag `--wait-strategy` will be introduced. It will have the options `Watcher` and `legacy`. `--wait-strategy` will default to watcher, which will use kstatus. +The Helm CLI will continue to use the existing `--wait` flag. The wait flag will be extended to accept `true|false|none|watcher|legacy`. Using `--wait` as flag with no argument, `--wait=true`, or `--wait=watcher` will use the kstatus watcher described in this document. Using `--wait=legacy` will use the current Helm 3 waiter. Using `--wait=false` or not using the `--wait` flag will not avoid waits. Kstatus can be used with either a poller or a watcher. The poller runs on a specified interval and only requires "list" RBAC permissions for polled resources. The watcher reacts to [watch events](https://github.com/kubernetes/kubernetes/blob/90a45563ae1bab5868ee888432fec9aac2f7f8b1/staging/src/k8s.io/apimachinery/pkg/watch/watch.go#L55-L61) and requires "list" and "watch" RBAC permissions. This proposal uses the watcher as it responds slightly faster when all resources are ready, and it is very likely that users applying or deleting resources will have "watch" permissions on their resources. From a9a5d17a86c56deb4eb92c68b279b655e533c8c3 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Fri, 14 Feb 2025 19:54:28 +0000 Subject: [PATCH 18/18] wait flag Signed-off-by: Austin Abro --- hips/hip-0999.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hips/hip-0999.md b/hips/hip-0999.md index 218b1c89..1bbe7dc1 100644 --- a/hips/hip-0999.md +++ b/hips/hip-0999.md @@ -25,7 +25,7 @@ Leveraging a existing status management library maintained by the Kubernetes tea ## Specification -The Helm CLI will continue to use the existing `--wait` flag. The wait flag will be extended to accept `true|false|none|watcher|legacy`. Using `--wait` as flag with no argument, `--wait=true`, or `--wait=watcher` will use the kstatus watcher described in this document. Using `--wait=legacy` will use the current Helm 3 waiter. Using `--wait=false` or not using the `--wait` flag will not avoid waits. +The Helm CLI will continue to use the existing `--wait` flag. The wait flag will be extended to accept `true|false|none|watcher|legacy`. When using `--wait` as flag with no argument, `--wait=true`, or `--wait=watcher` Helm will use the kstatus watcher described in this document. When using `--wait=legacy` Helm will use the current Helm 3 waiter. When using `--wait=false` or not using the `--wait` flag Helm not wait after deployments. Kstatus can be used with either a poller or a watcher. The poller runs on a specified interval and only requires "list" RBAC permissions for polled resources. The watcher reacts to [watch events](https://github.com/kubernetes/kubernetes/blob/90a45563ae1bab5868ee888432fec9aac2f7f8b1/staging/src/k8s.io/apimachinery/pkg/watch/watch.go#L55-L61) and requires "list" and "watch" RBAC permissions. This proposal uses the watcher as it responds slightly faster when all resources are ready, and it is very likely that users applying or deleting resources will have "watch" permissions on their resources.