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.

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

chore(plugins): set timeout for helm upgrade / install #884

Merged
merged 9 commits into from
Feb 7, 2025
2 changes: 1 addition & 1 deletion pkg/controllers/plugin/helm_chart_testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (r *PluginReconciler) reconcileHelmChartTest(ctx context.Context, plugin *g
return &reconcileResult{requeueAfter: result.requeueAfter}, nil
}

hasHelmChartTest, err := helm.HelmChartTest(ctx, restClientGetter, plugin)
hasHelmChartTest, err := helm.ChartTest(restClientGetter, plugin)
prometheusLabels := prometheus.Labels{
"cluster": plugin.Spec.ClusterName,
"plugin": plugin.Name,
Expand Down
6 changes: 4 additions & 2 deletions pkg/controllers/plugin/plugin_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ func (r *PluginReconciler) SetupWithManager(name string, mgr ctrl.Manager) error
WithOptions(controller.Options{
RateLimiter: workqueue.NewTypedMaxOfRateLimiter(
workqueue.NewTypedItemExponentialFailureRateLimiter[reconcile.Request](30*time.Second, 1*time.Hour),
&workqueue.TypedBucketRateLimiter[reconcile.Request]{Limiter: rate.NewLimiter(rate.Limit(10), 100)})}).
&workqueue.TypedBucketRateLimiter[reconcile.Request]{Limiter: rate.NewLimiter(rate.Limit(10), 100)}),
MaxConcurrentReconciles: 3,
}).
For(&greenhousev1alpha1.Plugin{}).
// If the release was (manually) modified the secret would have been modified. Reconcile it.
Watches(&corev1.Secret{},
Expand Down Expand Up @@ -168,7 +170,7 @@ func (r *PluginReconciler) EnsureCreated(ctx context.Context, resource lifecycle

reconcileErr := r.reconcileHelmRelease(ctx, restClientGetter, plugin, pluginDefinition)

// PluginStatus, WorkloadStatus and HelmChartTest should be reconciled regardless of Helm reconciliation result.
// PluginStatus, WorkloadStatus and ChartTest should be reconciled regardless of Helm reconciliation result.
r.reconcileStatus(ctx, restClientGetter, plugin, pluginDefinition, &plugin.Status)

workloadStatusResult, workloadStatusErr := r.reconcilePluginWorkloadStatus(ctx, restClientGetter, plugin, pluginDefinition)
Expand Down
13 changes: 9 additions & 4 deletions pkg/helm/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ var (

// IsHelmDebug is configured via a flag and enables extensive debug logging for Helm actions.
IsHelmDebug bool

//
abhijith-darshan marked this conversation as resolved.
Show resolved Hide resolved
installUpgradeTimeout = 150 * time.Second
abhijith-darshan marked this conversation as resolved.
Show resolved Hide resolved
)

// driftDetectionInterval is the interval after which a drift detection is performed.
Expand Down Expand Up @@ -112,8 +115,8 @@ func InstallOrUpgradeHelmChartFromPlugin(ctx context.Context, local client.Clien
return nil
}

// HelmChartTest to do helm test on the plugin
func HelmChartTest(ctx context.Context, restClientGetter genericclioptions.RESTClientGetter, plugin *greenhousev1alpha1.Plugin) (bool, error) {
// ChartTest to do helm test on the plugin
func ChartTest(restClientGetter genericclioptions.RESTClientGetter, plugin *greenhousev1alpha1.Plugin) (bool, error) {
var hasTestHook bool
cfg, err := newHelmAction(restClientGetter, plugin.Spec.ReleaseNamespace)
if err != nil {
Expand Down Expand Up @@ -231,7 +234,7 @@ func DiffChartToDeployedResources(ctx context.Context, local client.Client, rest
}

// ResetHelmReleaseStatusToDeployed resets the status of the release to deployed using a rollback.
func ResetHelmReleaseStatusToDeployed(ctx context.Context, restClientGetter genericclioptions.RESTClientGetter, plugin *greenhousev1alpha1.Plugin) error {
func ResetHelmReleaseStatusToDeployed(_ context.Context, restClientGetter genericclioptions.RESTClientGetter, plugin *greenhousev1alpha1.Plugin) error {
abhijith-darshan marked this conversation as resolved.
Show resolved Hide resolved
r, err := getLatestUpgradeableRelease(restClientGetter, plugin)
if err != nil {
return err
Expand Down Expand Up @@ -353,6 +356,7 @@ func upgradeRelease(ctx context.Context, local client.Client, restClientGetter g
upgradeAction.Namespace = plugin.Spec.ReleaseNamespace
upgradeAction.DependencyUpdate = true
upgradeAction.MaxHistory = 5
upgradeAction.Timeout = installUpgradeTimeout // set a timeout for the upgrade to not be stuck in pending state
upgradeAction.Description = pluginDefinition.Spec.Version

helmChart, err := loadHelmChart(&upgradeAction.ChartPathOptions, pluginDefinition.Spec.HelmChart, settings)
Expand Down Expand Up @@ -390,6 +394,7 @@ func installRelease(ctx context.Context, local client.Client, restClientGetter g
installAction := action.NewInstall(cfg)
installAction.ReleaseName = plugin.Name
installAction.Namespace = plugin.Spec.ReleaseNamespace
installAction.Timeout = installUpgradeTimeout // set a timeout for the installation to not be stuck in pending state
installAction.CreateNamespace = true
installAction.DependencyUpdate = true
installAction.DryRun = isDryRun
Expand Down Expand Up @@ -517,7 +522,7 @@ func mergeMaps(a, b map[string]interface{}) map[string]interface{} {

// getValuesForHelmChart returns a set of values to be used for Helm operations.
// The order is important as the values defined in the Helm chart can be overridden by the values defined in the Plugin.
func getValuesForHelmChart(ctx context.Context, c client.Client, helmChart *chart.Chart, plugin *greenhousev1alpha1.Plugin, isDryRun bool) (map[string]interface{}, error) {
func getValuesForHelmChart(ctx context.Context, c client.Client, helmChart *chart.Chart, plugin *greenhousev1alpha1.Plugin, _ bool) (map[string]interface{}, error) {
abhijith-darshan marked this conversation as resolved.
Show resolved Hide resolved
// Copy the values from the Helm chart ensuring a non-nil map.
helmValues := mergeMaps(make(map[string]interface{}, 0), helmChart.Values)
// Get values defined in plugin.
Expand Down
Loading