Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport release-1.28] Fix helm race condition #3649

Merged
merged 3 commits into from
Oct 26, 2023
Merged

[Backport release-1.28] Fix helm race condition #3649

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
293475c
Remove unused concurrencyLevel in helm controller
juanluisvaladas Oct 23, 2023
b0de723
Make extensions controller thread-safe
juanluisvaladas Oct 23, 2023
15c1c93
Implement timeouts in helm reconcile loop
juanluisvaladas Oct 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions cmd/controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,6 @@ func (c *command) start(ctx context.Context) error {
c.K0sVars,
adminClientFactory,
leaderElector,
// Hardcode until the config loading is fixed
10,
))
}

Expand Down
38 changes: 18 additions & 20 deletions pkg/component/controller/extensions_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
23 changes: 18 additions & 5 deletions pkg/helm/helm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package helm

import (
"context"
"fmt"
"os"
"path/filepath"
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down
Loading