From de3e6b47be4c7da000e935e38971109a211308d0 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Tue, 1 Oct 2024 17:47:20 +0000 Subject: [PATCH 01/11] dco Signed-off-by: Austin Abro --- src/internal/packager/helm/chart.go | 37 +++++++++++- src/pkg/packager/deploy.go | 31 +--------- src/pkg/packager/deploy_test.go | 89 ----------------------------- 3 files changed, 36 insertions(+), 121 deletions(-) diff --git a/src/internal/packager/helm/chart.go b/src/internal/packager/helm/chart.go index e1a8dc6c5a..4470db93b7 100644 --- a/src/internal/packager/helm/chart.go +++ b/src/internal/packager/helm/chart.go @@ -5,6 +5,7 @@ package helm import ( + "bytes" "context" "errors" "fmt" @@ -24,7 +25,9 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/yaml" + "github.com/zarf-dev/zarf/src/api/v1alpha1" "github.com/zarf-dev/zarf/src/config" + "github.com/zarf-dev/zarf/src/internal/packager/healthchecks" "github.com/zarf-dev/zarf/src/pkg/message" "github.com/zarf-dev/zarf/src/types" ) @@ -58,6 +61,7 @@ func (h *Helm) InstallOrUpgradeChart(ctx context.Context) (types.ConnectStrings, } histClient := action.NewHistory(h.actionConfig) + var release *release.Release err = retry.Do(func() error { var err error @@ -70,16 +74,15 @@ func (h *Helm) InstallOrUpgradeChart(ctx context.Context) (types.ConnectStrings, // No prior release, try to install it. spinner.Updatef("Attempting chart installation") - _, err = h.installChart(ctx, postRender) + release, err = h.installChart(ctx, postRender) } else if histErr == nil && len(releases) > 0 { // Otherwise, there is a prior release so upgrade it. spinner.Updatef("Attempting chart upgrade") lastRelease := releases[len(releases)-1] - _, err = h.upgradeChart(ctx, lastRelease, postRender) + release, err = h.upgradeChart(ctx, lastRelease, postRender) } else { - // 😭 things aren't working return fmt.Errorf("unable to verify the chart installation status: %w", histErr) } @@ -118,6 +121,34 @@ func (h *Helm) InstallOrUpgradeChart(ctx context.Context) (types.ConnectStrings, return nil, "", installErr } + resourceList, err := h.actionConfig.KubeClient.Build(bytes.NewBufferString(release.Manifest), true) + if err != nil { + return nil, "", fmt.Errorf("unable to build the resource list: %w", err) + } + + healthChecks := []v1alpha1.NamespacedObjectKindReference{} + for _, resource := range resourceList { + apiVersion, kind := resource.Object.GetObjectKind().GroupVersionKind().ToAPIVersionAndKind() + healthChecks = append(healthChecks, v1alpha1.NamespacedObjectKindReference{ + APIVersion: apiVersion, + Kind: kind, + Name: resource.Name, + Namespace: resource.Namespace, + }) + } + if !h.chart.NoWait { + // This re-uses the timeout from helm. This will increase the total amount of time a timeout can take + // However it is unlikely this step will take long + // The other option is to use the same context with as the helm install, this gets tricky with retries + healthChecksCtx, cancel := context.WithTimeout(ctx, h.timeout) + defer cancel() + spinner.Updatef("Running health checks") + if err := healthchecks.Run(healthChecksCtx, h.cluster.Watcher, healthChecks); err != nil { + return nil, "", err + } + } + spinner.Success() + // return any collected connect strings for zarf connect. return postRender.connectStrings, h.chart.ReleaseName, nil } diff --git a/src/pkg/packager/deploy.go b/src/pkg/packager/deploy.go index ee3796ac6c..1cb034dd38 100644 --- a/src/pkg/packager/deploy.go +++ b/src/pkg/packager/deploy.go @@ -19,19 +19,16 @@ import ( "golang.org/x/sync/errgroup" "github.com/avast/retry-go/v4" - pkgkubernetes "github.com/defenseunicorns/pkg/kubernetes" corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/cli-utils/pkg/kstatus/watcher" - "sigs.k8s.io/cli-utils/pkg/object" "github.com/defenseunicorns/pkg/helpers/v2" "github.com/zarf-dev/zarf/src/api/v1alpha1" "github.com/zarf-dev/zarf/src/config" "github.com/zarf-dev/zarf/src/internal/git" "github.com/zarf-dev/zarf/src/internal/gitea" + "github.com/zarf-dev/zarf/src/internal/packager/healthchecks" "github.com/zarf-dev/zarf/src/internal/packager/helm" "github.com/zarf-dev/zarf/src/internal/packager/images" "github.com/zarf-dev/zarf/src/internal/packager/template" @@ -238,30 +235,6 @@ func (p *Packager) deployComponents(ctx context.Context) ([]types.DeployedCompon return deployedComponents, nil } -func runHealthChecks(ctx context.Context, watcher watcher.StatusWatcher, healthChecks []v1alpha1.NamespacedObjectKindReference) error { - objs := []object.ObjMetadata{} - for _, hc := range healthChecks { - gv, err := schema.ParseGroupVersion(hc.APIVersion) - if err != nil { - return err - } - obj := object.ObjMetadata{ - GroupKind: schema.GroupKind{ - Group: gv.Group, - Kind: hc.Kind, - }, - Namespace: hc.Namespace, - Name: hc.Name, - } - objs = append(objs, obj) - } - err := pkgkubernetes.WaitForReady(ctx, watcher, objs) - if err != nil { - return err - } - return nil -} - func (p *Packager) deployInitComponent(ctx context.Context, component v1alpha1.ZarfComponent) ([]types.InstalledChart, error) { hasExternalRegistry := p.cfg.InitOpts.RegistryInfo.Address != "" isSeedRegistry := component.Name == "zarf-seed-registry" @@ -403,7 +376,7 @@ func (p *Packager) deployComponent(ctx context.Context, component v1alpha1.ZarfC defer cancel() spinner := message.NewProgressSpinner("Running health checks") defer spinner.Stop() - if err = runHealthChecks(healthCheckContext, p.cluster.Watcher, component.HealthChecks); err != nil { + if err = healthchecks.Run(healthCheckContext, p.cluster.Watcher, component.HealthChecks); err != nil { return nil, fmt.Errorf("health checks failed: %w", err) } spinner.Success() diff --git a/src/pkg/packager/deploy_test.go b/src/pkg/packager/deploy_test.go index f5dba049e1..a1f32aa346 100644 --- a/src/pkg/packager/deploy_test.go +++ b/src/pkg/packager/deploy_test.go @@ -4,22 +4,12 @@ package packager import ( - "context" "testing" - "time" "github.com/stretchr/testify/require" "github.com/zarf-dev/zarf/src/api/v1alpha1" "github.com/zarf-dev/zarf/src/pkg/packager/sources" "github.com/zarf-dev/zarf/src/types" - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/yaml" - dynamicfake "k8s.io/client-go/dynamic/fake" - "k8s.io/kubectl/pkg/scheme" - "sigs.k8s.io/cli-utils/pkg/kstatus/watcher" - "sigs.k8s.io/cli-utils/pkg/testutil" ) func TestGenerateValuesOverrides(t *testing.T) { @@ -282,82 +272,3 @@ func TestServiceInfoFromServiceURL(t *testing.T) { }) } } - -var podCurrentYaml = ` -apiVersion: v1 -kind: Pod -metadata: - name: good-pod - namespace: ns -status: - conditions: - - type: Ready - status: "True" - phase: Running -` - -var podYaml = ` -apiVersion: v1 -kind: Pod -metadata: - name: in-progress-pod - namespace: ns -` - -func yamlToUnstructured(t *testing.T, yml string) *unstructured.Unstructured { - t.Helper() - m := make(map[string]interface{}) - err := yaml.Unmarshal([]byte(yml), &m) - require.NoError(t, err) - return &unstructured.Unstructured{Object: m} -} - -func TestRunHealthChecks(t *testing.T) { - t.Parallel() - tests := []struct { - name string - podYaml string - expectErr error - }{ - { - name: "Pod is running", - podYaml: podCurrentYaml, - expectErr: nil, - }, - { - name: "Pod is never ready", - podYaml: podYaml, - expectErr: context.DeadlineExceeded, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - fakeClient := dynamicfake.NewSimpleDynamicClient(scheme.Scheme) - fakeMapper := testutil.NewFakeRESTMapper( - v1.SchemeGroupVersion.WithKind("Pod"), - ) - ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) - defer cancel() - pod := yamlToUnstructured(t, tt.podYaml) - statusWatcher := watcher.NewDefaultStatusWatcher(fakeClient, fakeMapper) - podGVR := schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"} - require.NoError(t, fakeClient.Tracker().Create(podGVR, pod, pod.GetNamespace())) - objs := []v1alpha1.NamespacedObjectKindReference{ - { - APIVersion: pod.GetAPIVersion(), - Kind: pod.GetKind(), - Namespace: pod.GetNamespace(), - Name: pod.GetName(), - }, - } - err := runHealthChecks(ctx, statusWatcher, objs) - if tt.expectErr != nil { - require.ErrorIs(t, err, tt.expectErr) - return - } - require.NoError(t, err) - }) - } -} From a5e1798c55ceeca037c7c00b5aa085cead013d5f Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Tue, 1 Oct 2024 17:47:49 +0000 Subject: [PATCH 02/11] add health checks module Signed-off-by: Austin Abro --- .../packager/healthchecks/healthchecks.go | 40 +++++++ .../healthchecks/healthchecks_test.go | 101 ++++++++++++++++++ 2 files changed, 141 insertions(+) create mode 100644 src/internal/packager/healthchecks/healthchecks.go create mode 100644 src/internal/packager/healthchecks/healthchecks_test.go diff --git a/src/internal/packager/healthchecks/healthchecks.go b/src/internal/packager/healthchecks/healthchecks.go new file mode 100644 index 0000000000..7957953378 --- /dev/null +++ b/src/internal/packager/healthchecks/healthchecks.go @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +// Package healthchecks run kstatus style health checks on a list of objects +package healthchecks + +import ( + "context" + + pkgkubernetes "github.com/defenseunicorns/pkg/kubernetes" + "github.com/zarf-dev/zarf/src/api/v1alpha1" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/cli-utils/pkg/kstatus/watcher" + "sigs.k8s.io/cli-utils/pkg/object" +) + +// Run waits for a list of objects to be reconciled +func Run(ctx context.Context, watcher watcher.StatusWatcher, healthChecks []v1alpha1.NamespacedObjectKindReference) error { + objs := []object.ObjMetadata{} + for _, hc := range healthChecks { + gv, err := schema.ParseGroupVersion(hc.APIVersion) + if err != nil { + return err + } + obj := object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: gv.Group, + Kind: hc.Kind, + }, + Namespace: hc.Namespace, + Name: hc.Name, + } + objs = append(objs, obj) + } + err := pkgkubernetes.WaitForReady(ctx, watcher, objs) + if err != nil { + return err + } + return nil +} diff --git a/src/internal/packager/healthchecks/healthchecks_test.go b/src/internal/packager/healthchecks/healthchecks_test.go new file mode 100644 index 0000000000..d8a0b27c1f --- /dev/null +++ b/src/internal/packager/healthchecks/healthchecks_test.go @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +// Package healthchecks run kstatus style health checks on a list of objects +package healthchecks + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + "github.com/zarf-dev/zarf/src/api/v1alpha1" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/yaml" + dynamicfake "k8s.io/client-go/dynamic/fake" + "k8s.io/kubectl/pkg/scheme" + "sigs.k8s.io/cli-utils/pkg/kstatus/watcher" + "sigs.k8s.io/cli-utils/pkg/testutil" +) + +var podCurrentYaml = ` +apiVersion: v1 +kind: Pod +metadata: + name: good-pod + namespace: ns +status: + conditions: + - type: Ready + status: "True" + phase: Running +` + +var podYaml = ` +apiVersion: v1 +kind: Pod +metadata: + name: in-progress-pod + namespace: ns +` + +func yamlToUnstructured(t *testing.T, yml string) *unstructured.Unstructured { + t.Helper() + m := make(map[string]interface{}) + err := yaml.Unmarshal([]byte(yml), &m) + require.NoError(t, err) + return &unstructured.Unstructured{Object: m} +} + +func TestRunHealthChecks(t *testing.T) { + t.Parallel() + tests := []struct { + name string + podYaml string + expectErr error + }{ + { + name: "Pod is running", + podYaml: podCurrentYaml, + expectErr: nil, + }, + { + name: "Pod is never ready", + podYaml: podYaml, + expectErr: context.DeadlineExceeded, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + fakeClient := dynamicfake.NewSimpleDynamicClient(scheme.Scheme) + fakeMapper := testutil.NewFakeRESTMapper( + v1.SchemeGroupVersion.WithKind("Pod"), + ) + ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) + defer cancel() + pod := yamlToUnstructured(t, tt.podYaml) + statusWatcher := watcher.NewDefaultStatusWatcher(fakeClient, fakeMapper) + podGVR := schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"} + require.NoError(t, fakeClient.Tracker().Create(podGVR, pod, pod.GetNamespace())) + objs := []v1alpha1.NamespacedObjectKindReference{ + { + APIVersion: pod.GetAPIVersion(), + Kind: pod.GetKind(), + Namespace: pod.GetNamespace(), + Name: pod.GetName(), + }, + } + err := Run(ctx, statusWatcher, objs) + if tt.expectErr != nil { + require.ErrorIs(t, err, tt.expectErr) + return + } + require.NoError(t, err) + }) + } +} From 68ac94e186df43105d0762afecb3017ff303428c Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Tue, 1 Oct 2024 17:49:26 +0000 Subject: [PATCH 03/11] revise comment Signed-off-by: Austin Abro --- src/internal/packager/helm/chart.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/internal/packager/helm/chart.go b/src/internal/packager/helm/chart.go index 4470db93b7..8399b4cab5 100644 --- a/src/internal/packager/helm/chart.go +++ b/src/internal/packager/helm/chart.go @@ -139,7 +139,6 @@ func (h *Helm) InstallOrUpgradeChart(ctx context.Context) (types.ConnectStrings, if !h.chart.NoWait { // This re-uses the timeout from helm. This will increase the total amount of time a timeout can take // However it is unlikely this step will take long - // The other option is to use the same context with as the helm install, this gets tricky with retries healthChecksCtx, cancel := context.WithTimeout(ctx, h.timeout) defer cancel() spinner.Updatef("Running health checks") From d71397946f9adede72a1dd0d364d3fbf1025aa2a Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Wed, 2 Oct 2024 14:10:58 +0000 Subject: [PATCH 04/11] update docs Signed-off-by: Austin Abro --- site/src/content/docs/ref/deploy.mdx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/site/src/content/docs/ref/deploy.mdx b/site/src/content/docs/ref/deploy.mdx index 0e700b0083..5fe6fe999a 100644 --- a/site/src/content/docs/ref/deploy.mdx +++ b/site/src/content/docs/ref/deploy.mdx @@ -146,6 +146,8 @@ Deployments will wait for helm [post-install hooks](https://helm.sh/docs/topics/ ::: +After the Helm wait completes successfully, Zarf waits for all resources in the applied chart to fully reconcile. To determine when reconciliation is achieved, Zarf uses [kstatus](https://github.com/kubernetes-sigs/cli-utils/blob/master/pkg/kstatus/README.md#kstatus). Kstatus determines when a resource is reconciled using the [status](https://kubernetes.io/docs/concepts/overview/working-with-objects/#object-spec-and-status) field. If a resource does not have a status field, kstatus considers it reconciled once it's found. + ### Timeout Settings The default timeout for Helm operations in Zarf is 15 minutes. From a5cbe62570e8714a1d59ea1f38d43fab909aafe8 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Wed, 2 Oct 2024 14:28:12 +0000 Subject: [PATCH 05/11] change timeout logic Signed-off-by: Austin Abro --- src/internal/packager/helm/chart.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/internal/packager/helm/chart.go b/src/internal/packager/helm/chart.go index 8399b4cab5..c71dee1c28 100644 --- a/src/internal/packager/helm/chart.go +++ b/src/internal/packager/helm/chart.go @@ -63,12 +63,14 @@ func (h *Helm) InstallOrUpgradeChart(ctx context.Context) (types.ConnectStrings, histClient := action.NewHistory(h.actionConfig) var release *release.Release + var helmOpStart time.Time err = retry.Do(func() error { var err error releases, histErr := histClient.Run(h.chart.ReleaseName) spinner.Updatef("Checking for existing helm deployment") + helmOpStart = time.Now() if errors.Is(histErr, driver.ErrReleaseNotFound) { // No prior release, try to install it. @@ -137,9 +139,9 @@ func (h *Helm) InstallOrUpgradeChart(ctx context.Context) (types.ConnectStrings, }) } if !h.chart.NoWait { - // This re-uses the timeout from helm. This will increase the total amount of time a timeout can take - // However it is unlikely this step will take long - healthChecksCtx, cancel := context.WithTimeout(ctx, h.timeout) + // Ensure we don't go past the timeout by getting the time since the helm operation started + healthCheckTimeout := h.timeout - time.Since(helmOpStart) + healthChecksCtx, cancel := context.WithTimeout(ctx, healthCheckTimeout) defer cancel() spinner.Updatef("Running health checks") if err := healthchecks.Run(healthChecksCtx, h.cluster.Watcher, healthChecks); err != nil { From 0871213521df58e99ea812b5c843e06dbaaad931 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Wed, 2 Oct 2024 16:34:03 +0000 Subject: [PATCH 06/11] inline function Signed-off-by: Austin Abro --- .../packager/healthchecks/healthchecks_test.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/internal/packager/healthchecks/healthchecks_test.go b/src/internal/packager/healthchecks/healthchecks_test.go index d8a0b27c1f..9761f7ba84 100644 --- a/src/internal/packager/healthchecks/healthchecks_test.go +++ b/src/internal/packager/healthchecks/healthchecks_test.go @@ -42,14 +42,6 @@ metadata: namespace: ns ` -func yamlToUnstructured(t *testing.T, yml string) *unstructured.Unstructured { - t.Helper() - m := make(map[string]interface{}) - err := yaml.Unmarshal([]byte(yml), &m) - require.NoError(t, err) - return &unstructured.Unstructured{Object: m} -} - func TestRunHealthChecks(t *testing.T) { t.Parallel() tests := []struct { @@ -78,7 +70,10 @@ func TestRunHealthChecks(t *testing.T) { ) ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) defer cancel() - pod := yamlToUnstructured(t, tt.podYaml) + m := make(map[string]interface{}) + err := yaml.Unmarshal([]byte(tt.podYaml), &m) + require.NoError(t, err) + pod := &unstructured.Unstructured{Object: m} statusWatcher := watcher.NewDefaultStatusWatcher(fakeClient, fakeMapper) podGVR := schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"} require.NoError(t, fakeClient.Tracker().Create(podGVR, pod, pod.GetNamespace())) @@ -90,7 +85,7 @@ func TestRunHealthChecks(t *testing.T) { Name: pod.GetName(), }, } - err := Run(ctx, statusWatcher, objs) + err = Run(ctx, statusWatcher, objs) if tt.expectErr != nil { require.ErrorIs(t, err, tt.expectErr) return From 48057fbf6d63f98a4728257e3243c63b63557bb2 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Wed, 2 Oct 2024 16:34:54 +0000 Subject: [PATCH 07/11] move healthchecks outside of packager Signed-off-by: Austin Abro --- src/internal/{packager => }/healthchecks/healthchecks.go | 0 src/internal/{packager => }/healthchecks/healthchecks_test.go | 0 src/internal/packager/helm/chart.go | 2 +- src/pkg/packager/deploy.go | 2 +- 4 files changed, 2 insertions(+), 2 deletions(-) rename src/internal/{packager => }/healthchecks/healthchecks.go (100%) rename src/internal/{packager => }/healthchecks/healthchecks_test.go (100%) diff --git a/src/internal/packager/healthchecks/healthchecks.go b/src/internal/healthchecks/healthchecks.go similarity index 100% rename from src/internal/packager/healthchecks/healthchecks.go rename to src/internal/healthchecks/healthchecks.go diff --git a/src/internal/packager/healthchecks/healthchecks_test.go b/src/internal/healthchecks/healthchecks_test.go similarity index 100% rename from src/internal/packager/healthchecks/healthchecks_test.go rename to src/internal/healthchecks/healthchecks_test.go diff --git a/src/internal/packager/helm/chart.go b/src/internal/packager/helm/chart.go index c71dee1c28..d813c17dec 100644 --- a/src/internal/packager/helm/chart.go +++ b/src/internal/packager/helm/chart.go @@ -27,7 +27,7 @@ import ( "github.com/zarf-dev/zarf/src/api/v1alpha1" "github.com/zarf-dev/zarf/src/config" - "github.com/zarf-dev/zarf/src/internal/packager/healthchecks" + "github.com/zarf-dev/zarf/src/internal/healthchecks" "github.com/zarf-dev/zarf/src/pkg/message" "github.com/zarf-dev/zarf/src/types" ) diff --git a/src/pkg/packager/deploy.go b/src/pkg/packager/deploy.go index 1cb034dd38..b7ca9014fa 100644 --- a/src/pkg/packager/deploy.go +++ b/src/pkg/packager/deploy.go @@ -28,7 +28,7 @@ import ( "github.com/zarf-dev/zarf/src/config" "github.com/zarf-dev/zarf/src/internal/git" "github.com/zarf-dev/zarf/src/internal/gitea" - "github.com/zarf-dev/zarf/src/internal/packager/healthchecks" + "github.com/zarf-dev/zarf/src/internal/healthchecks" "github.com/zarf-dev/zarf/src/internal/packager/helm" "github.com/zarf-dev/zarf/src/internal/packager/images" "github.com/zarf-dev/zarf/src/internal/packager/template" From 093ddf1b0e4dc8173b68d62cbc49abd112379abf Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Wed, 2 Oct 2024 16:39:25 +0000 Subject: [PATCH 08/11] update status Signed-off-by: Austin Abro --- site/src/content/docs/ref/deploy.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/content/docs/ref/deploy.mdx b/site/src/content/docs/ref/deploy.mdx index 5fe6fe999a..6286fd350f 100644 --- a/site/src/content/docs/ref/deploy.mdx +++ b/site/src/content/docs/ref/deploy.mdx @@ -146,7 +146,7 @@ Deployments will wait for helm [post-install hooks](https://helm.sh/docs/topics/ ::: -After the Helm wait completes successfully, Zarf waits for all resources in the applied chart to fully reconcile. To determine when reconciliation is achieved, Zarf uses [kstatus](https://github.com/kubernetes-sigs/cli-utils/blob/master/pkg/kstatus/README.md#kstatus). Kstatus determines when a resource is reconciled using the [status](https://kubernetes.io/docs/concepts/overview/working-with-objects/#object-spec-and-status) field. If a resource does not have a status field, kstatus considers it reconciled once it's found. +After the Helm wait completes successfully, Zarf waits for all resources in the applied chart to fully reconcile. To identify when reconciliation is achieved, Zarf uses [kstatus](https://github.com/kubernetes-sigs/cli-utils/blob/master/pkg/kstatus/README.md#kstatus). Kstatus assesses whether a resource is reconciled by checking the [status](https://kubernetes.io/docs/concepts/overview/working-with-objects/#object-spec-and-status) field. If a resource does not have a status field, kstatus considers it reconciled once it's found. ### Timeout Settings From 18ce5a0767b00e8b259f871094ece5f8e682f237 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Thu, 3 Oct 2024 13:16:03 +0000 Subject: [PATCH 09/11] use context in helm over timeout Signed-off-by: Austin Abro --- src/internal/packager/helm/chart.go | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/internal/packager/helm/chart.go b/src/internal/packager/helm/chart.go index d813c17dec..cb04e11e8c 100644 --- a/src/internal/packager/helm/chart.go +++ b/src/internal/packager/helm/chart.go @@ -63,27 +63,32 @@ func (h *Helm) InstallOrUpgradeChart(ctx context.Context) (types.ConnectStrings, histClient := action.NewHistory(h.actionConfig) var release *release.Release - var helmOpStart time.Time + var helmCtx context.Context + var helmCtxCancel context.CancelFunc + err = retry.Do(func() error { var err error releases, histErr := histClient.Run(h.chart.ReleaseName) spinner.Updatef("Checking for existing helm deployment") - helmOpStart = time.Now() - + // cancel context from previous retry if it exists + if helmCtxCancel != nil { + helmCtxCancel() + } + helmCtx, helmCtxCancel = context.WithTimeout(ctx, h.timeout) if errors.Is(histErr, driver.ErrReleaseNotFound) { // No prior release, try to install it. spinner.Updatef("Attempting chart installation") - release, err = h.installChart(ctx, postRender) + release, err = h.installChart(helmCtx, postRender) } else if histErr == nil && len(releases) > 0 { // Otherwise, there is a prior release so upgrade it. spinner.Updatef("Attempting chart upgrade") lastRelease := releases[len(releases)-1] - release, err = h.upgradeChart(ctx, lastRelease, postRender) + release, err = h.upgradeChart(helmCtx, lastRelease, postRender) } else { return fmt.Errorf("unable to verify the chart installation status: %w", histErr) } @@ -95,6 +100,7 @@ func (h *Helm) InstallOrUpgradeChart(ctx context.Context) (types.ConnectStrings, spinner.Success() return nil }, retry.Context(ctx), retry.Attempts(uint(h.retries)), retry.Delay(500*time.Millisecond)) + defer helmCtxCancel() if err != nil { removeMsg := "if you need to remove the failed chart, use `zarf package remove`" installErr := fmt.Errorf("unable to install chart after %d attempts: %w: %s", h.retries, err, removeMsg) @@ -139,12 +145,9 @@ func (h *Helm) InstallOrUpgradeChart(ctx context.Context) (types.ConnectStrings, }) } if !h.chart.NoWait { - // Ensure we don't go past the timeout by getting the time since the helm operation started - healthCheckTimeout := h.timeout - time.Since(helmOpStart) - healthChecksCtx, cancel := context.WithTimeout(ctx, healthCheckTimeout) - defer cancel() + // Ensure we don't go past the timeout by using a context initialized with the helm timeout spinner.Updatef("Running health checks") - if err := healthchecks.Run(healthChecksCtx, h.cluster.Watcher, healthChecks); err != nil { + if err := healthchecks.Run(helmCtx, h.cluster.Watcher, healthChecks); err != nil { return nil, "", err } } From 2bbdfd5e72464b403caa7a8ff6b47e77ae3ed1d2 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Tue, 8 Oct 2024 18:08:15 +0000 Subject: [PATCH 10/11] simplify context Signed-off-by: Austin Abro --- src/internal/packager/helm/chart.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/internal/packager/helm/chart.go b/src/internal/packager/helm/chart.go index cb04e11e8c..8057661d4c 100644 --- a/src/internal/packager/helm/chart.go +++ b/src/internal/packager/helm/chart.go @@ -63,8 +63,8 @@ func (h *Helm) InstallOrUpgradeChart(ctx context.Context) (types.ConnectStrings, histClient := action.NewHistory(h.actionConfig) var release *release.Release - var helmCtx context.Context - var helmCtxCancel context.CancelFunc + helmCtx, helmCtxCancel := context.WithTimeout(ctx, h.timeout) + defer helmCtxCancel() err = retry.Do(func() error { var err error @@ -72,11 +72,7 @@ func (h *Helm) InstallOrUpgradeChart(ctx context.Context) (types.ConnectStrings, releases, histErr := histClient.Run(h.chart.ReleaseName) spinner.Updatef("Checking for existing helm deployment") - // cancel context from previous retry if it exists - if helmCtxCancel != nil { - helmCtxCancel() - } - helmCtx, helmCtxCancel = context.WithTimeout(ctx, h.timeout) + if errors.Is(histErr, driver.ErrReleaseNotFound) { // No prior release, try to install it. spinner.Updatef("Attempting chart installation") From c81625141061bc8062bb027fdeffac64e3301f66 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Tue, 8 Oct 2024 20:07:27 +0000 Subject: [PATCH 11/11] don't cancel twice Signed-off-by: Austin Abro --- src/internal/packager/helm/chart.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/internal/packager/helm/chart.go b/src/internal/packager/helm/chart.go index 8057661d4c..6009d5f9bc 100644 --- a/src/internal/packager/helm/chart.go +++ b/src/internal/packager/helm/chart.go @@ -96,7 +96,6 @@ func (h *Helm) InstallOrUpgradeChart(ctx context.Context) (types.ConnectStrings, spinner.Success() return nil }, retry.Context(ctx), retry.Attempts(uint(h.retries)), retry.Delay(500*time.Millisecond)) - defer helmCtxCancel() if err != nil { removeMsg := "if you need to remove the failed chart, use `zarf package remove`" installErr := fmt.Errorf("unable to install chart after %d attempts: %w: %s", h.retries, err, removeMsg)