From 8d0b54e431eeb3a256a36b5e71ecb5a4694ce7d6 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 15 Dec 2020 00:55:48 +0100 Subject: [PATCH] Make proper use of errgroup context Signed-off-by: Hidde Beydals --- controllers/helmchart_controller.go | 2 +- internal/helm/dependency_manager.go | 10 +++++++--- internal/helm/dependency_manager_test.go | 9 +++++---- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index db312f96..ec75e1d2 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -589,7 +589,7 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context, Chart: helmChart, Dependencies: dwr, } - err = dm.Build() + err = dm.Build(ctx) if err != nil { return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err } diff --git a/internal/helm/dependency_manager.go b/internal/helm/dependency_manager.go index 55ebc337..5ecd5899 100644 --- a/internal/helm/dependency_manager.go +++ b/internal/helm/dependency_manager.go @@ -46,16 +46,20 @@ type DependencyManager struct { } // Build compiles and builds the chart dependencies -func (dm *DependencyManager) Build() error { +func (dm *DependencyManager) Build(ctx context.Context) error { if len(dm.Dependencies) == 0 { return nil } - ctx := context.Background() errs, ctx := errgroup.WithContext(ctx) - for _, item := range dm.Dependencies { errs.Go(func() error { + select { + case <-ctx.Done(): + return ctx.Err() + default: + } + var ( ch *helmchart.Chart err error diff --git a/internal/helm/dependency_manager_test.go b/internal/helm/dependency_manager_test.go index ae948537..5f55c2c1 100644 --- a/internal/helm/dependency_manager_test.go +++ b/internal/helm/dependency_manager_test.go @@ -17,6 +17,7 @@ limitations under the License. package helm import ( + "context" "fmt" "io/ioutil" "strings" @@ -43,7 +44,7 @@ func TestBuild_WithEmptyDependencies(t *testing.T) { dm := DependencyManager{ Dependencies: nil, } - if err := dm.Build(); err != nil { + if err := dm.Build(context.TODO()); err != nil { t.Errorf("Build() should return nil") } } @@ -119,7 +120,7 @@ func TestBuild_WithLocalChart(t *testing.T) { }, } - err := dm.Build() + err := dm.Build(context.TODO()) deps := dm.Chart.Dependencies() if (err != nil) && tt.wantErr { @@ -172,7 +173,7 @@ func TestBuild_WithRemoteChart(t *testing.T) { }, } - if err := dm.Build(); err != nil { + if err := dm.Build(context.TODO()); err != nil { t.Errorf("Build() expected to not return error: %s", err) } @@ -189,7 +190,7 @@ func TestBuild_WithRemoteChart(t *testing.T) { // When repo is not set dm.Dependencies[0].Repo = nil - if err := dm.Build(); err == nil { + if err := dm.Build(context.TODO()); err == nil { t.Errorf("Build() expected to return error") } else if !strings.Contains(err.Error(), "chartrepo should not be nil") { t.Errorf("Build() expected to return different error, got: %s", err)