Merge pull request #3633 from juanluisvaladas/fix-helm-race-condition

Fix helm race condition

cmd/controller/controller.go

@@ -373,8 +373,6 @@ func (c *command) start(ctx context.Context) error {
 			c.K0sVars,
 			adminClientFactory,
 			leaderElector,
-			// Hardcode until the config loading is fixed
-			10,
 		))
 	}
 

pkg/component/controller/extensions_controller.go

@@ -52,26 +52,24 @@ import (
 
 // Helm watch for Chart crd
 type ExtensionsController struct {
-	concurrencyLevel int
-	saver            manifestsSaver
-	L                *logrus.Entry
-	helm             *helm.Commands
-	kubeConfig       string
-	leaderElector    leaderelector.Interface
+	saver         manifestsSaver
+	L             *logrus.Entry
+	helm          *helm.Commands
+	kubeConfig    string
+	leaderElector leaderelector.Interface
 }
 
 var _ manager.Component = (*ExtensionsController)(nil)
 var _ manager.Reconciler = (*ExtensionsController)(nil)
 
 // NewExtensionsController builds new HelmAddons
-func NewExtensionsController(s manifestsSaver, k0sVars *config.CfgVars, kubeClientFactory kubeutil.ClientFactoryInterface, leaderElector leaderelector.Interface, concurrencyLevel int) *ExtensionsController {
+func NewExtensionsController(s manifestsSaver, k0sVars *config.CfgVars, kubeClientFactory kubeutil.ClientFactoryInterface, leaderElector leaderelector.Interface) *ExtensionsController {
 	return &ExtensionsController{
-		concurrencyLevel: concurrencyLevel,
-		saver:            s,
-		L:                logrus.WithFields(logrus.Fields{"component": "extensions_controller"}),
-		helm:             helm.NewCommands(k0sVars),
-		kubeConfig:       k0sVars.AdminKubeConfigPath,
-		leaderElector:    leaderElector,
+		saver:         s,
+		L:             logrus.WithFields(logrus.Fields{"component": "extensions_controller"}),
+		helm:          helm.NewCommands(k0sVars),
+		kubeConfig:    k0sVars.AdminKubeConfigPath,
+		leaderElector: leaderElector,
 	}
 }
 
@@ -226,7 +224,7 @@ func (cr *ChartReconciler) Reconcile(ctx context.Context, req reconcile.Request)
 	return reconcile.Result{}, nil
 }
 func (cr *ChartReconciler) uninstall(ctx context.Context, chart v1beta1.Chart) error {
-	if err := cr.helm.UninstallRelease(chart.Status.ReleaseName, chart.Status.Namespace); err != nil {
+	if err := cr.helm.UninstallRelease(ctx, chart.Status.ReleaseName, chart.Status.Namespace); err != nil {
 		return fmt.Errorf("can't uninstall release `%s/%s`: %w", chart.Status.Namespace, chart.Status.ReleaseName, err)
 	}
 	return nil
@@ -254,7 +252,8 @@ func (cr *ChartReconciler) updateOrInstallChart(ctx context.Context, chart v1bet
 	if chart.Status.ReleaseName == "" {
 		// new chartRelease
 		cr.L.Tracef("Start update or install %s", chart.Spec.ChartName)
-		chartRelease, err = cr.helm.InstallChart(chart.Spec.ChartName,
+		chartRelease, err = cr.helm.InstallChart(ctx,
+			chart.Spec.ChartName,
 			chart.Spec.Version,
 			chart.Spec.ReleaseName,
 			chart.Spec.Namespace,
@@ -267,7 +266,8 @@ func (cr *ChartReconciler) updateOrInstallChart(ctx context.Context, chart v1bet
 	} else {
 		if cr.chartNeedsUpgrade(chart) {
 			// update
-			chartRelease, err = cr.helm.UpgradeChart(chart.Spec.ChartName,
+			chartRelease, err = cr.helm.UpgradeChart(ctx,
+				chart.Spec.ChartName,
 				chart.Spec.Version,
 				chart.Status.ReleaseName,
 				chart.Status.Namespace,
@@ -356,10 +356,8 @@ func (ec *ExtensionsController) Start(ctx context.Context) error {
 		Metrics: metricsserver.Options{
 			BindAddress: "0",
 		},
-		Logger: logrusr.New(ec.L),
-		Controller: ctrlconfig.Controller{
-			GroupKindConcurrency: map[string]int{gk.String(): 10},
-		},
+		Logger:     logrusr.New(ec.L),
+		Controller: ctrlconfig.Controller{},
 	})
 	if err != nil {
 		return fmt.Errorf("can't build controller-runtime controller for helm extensions: %w", err)

pkg/helm/helm.go

@@ -17,6 +17,7 @@ limitations under the License.
 package helm
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -41,6 +42,7 @@ import (
 )
 
 // Commands run different helm command in the same way as CLI tool
+// This struct isn't thread-safe. Check on a per function basis.
 type Commands struct {
 	repoFile     string
 	helmCacheDir string
@@ -206,7 +208,9 @@ func (hc *Commands) isInstallable(chart *chart.Chart) bool {
 	return true
 }
 
-func (hc *Commands) InstallChart(chartName string, version string, releaseName string, namespace string, values map[string]interface{}, timeout time.Duration) (*release.Release, error) {
+// InstallChart installs a helm chart
+// InstallChart, UpgradeChart and UninstallRelease(releaseName are *NOT* thread-safe
+func (hc *Commands) InstallChart(ctx context.Context, chartName string, version string, releaseName string, namespace string, values map[string]interface{}, timeout time.Duration) (*release.Release, error) {
 	cfg, err := hc.getActionCfg(namespace)
 	if err != nil {
 		return nil, fmt.Errorf("can't create action configuration: %v", err)
@@ -246,14 +250,16 @@ func (hc *Commands) InstallChart(chartName string, version string, releaseName s
 	if err != nil {
 		return nil, fmt.Errorf("can't reload loadedChart `%s`: %v", chartDir, err)
 	}
-	chartRelease, err := install.Run(loadedChart, values)
+	chartRelease, err := install.RunWithContext(ctx, loadedChart, values)
 	if err != nil {
 		return nil, fmt.Errorf("can't install loadedChart `%s`: %v", loadedChart.Name(), err)
 	}
 	return chartRelease, nil
 }
 
-func (hc *Commands) UpgradeChart(chartName string, version string, releaseName string, namespace string, values map[string]interface{}, timeout time.Duration) (*release.Release, error) {
+// UpgradeChart upgrades a helm chart.
+// InstallChart, UpgradeChart and UninstallRelease(releaseName are *NOT* thread-safe
+func (hc *Commands) UpgradeChart(ctx context.Context, chartName string, version string, releaseName string, namespace string, values map[string]interface{}, timeout time.Duration) (*release.Release, error) {
 	cfg, err := hc.getActionCfg(namespace)
 	if err != nil {
 		return nil, fmt.Errorf("can't create action configuration: %v", err)
@@ -287,7 +293,7 @@ func (hc *Commands) UpgradeChart(chartName string, version string, releaseName s
 		return nil, fmt.Errorf("can't reload loadedChart `%s`: %v", chartDir, err)
 	}
 
-	chartRelease, err := upgrade.Run(releaseName, loadedChart, values)
+	chartRelease, err := upgrade.RunWithContext(ctx, releaseName, loadedChart, values)
 	if err != nil {
 		return nil, fmt.Errorf("can't upgrade loadedChart `%s`: %v", loadedChart.Metadata.Name, err)
 	}
@@ -308,12 +314,19 @@ func (hc *Commands) ListReleases(namespace string) ([]*release.Release, error) {
 	return helmAction.Run()
 }
 
-func (hc *Commands) UninstallRelease(releaseName string, namespace string) error {
+// UninstallRelease uninstalls a release.
+// InstallChart, UpgradeChart and UninstallRelease(releaseName are *NOT* thread-safe
+func (hc *Commands) UninstallRelease(ctx context.Context, releaseName string, namespace string) error {
 	cfg, err := hc.getActionCfg(namespace)
 	if err != nil {
 		return fmt.Errorf("can't create helmAction configuration: %v", err)
 	}
 	helmAction := action.NewUninstall(cfg)
+	deadline, ok := ctx.Deadline()
+	if ok {
+		helmAction.Timeout = time.Until(deadline)
+	}
+
 	if _, err := helmAction.Run(releaseName); err != nil {
 		return fmt.Errorf("can't uninstall release `%s`: %v", releaseName, err)
 	}