2
\$\begingroup\$

I use the following code which works and install helm chart (in the loop) in parallel, it get a kubeconfig file (to access to k8s) create a helm client and install the charts from the defined location and waits (upgradeAction.Wait = true) that each chart installation will finished successfully.

At first I do it in sequence and now I added a wait.group to be able to reduce the installation time and install it in parallel, do I miss something? error handling etc?

    package main
    
    import (
        "fmt"
        "os"
        "strings"
        "sync"
    )
    
    func main() {
      log.Info("tools Installation")

    kcfgFilePath, err := kubeFile(cfg)
    defer os.Remove(kcfgFilePath)
    if err != nil {
        return err
    }
    settings := cli.New()
    actionConfig := new(action.Configuration)
    clientGetter := genericclioptions.NewConfigFlags(false)
    clientGetter.KubeConfig = &kcfgFilePath
    // install all relevant components
    var wg sync.WaitGroup

    for _, chartInstallation := range charts {
        wg.Add(1)
         go installChart(&wg,chartInstallation, releaseName, actionConfig, clientGetter, settings, log)
    }
    return nil
}

func installChart(wg *sync.WaitGroup,chartInstallation Installation, releaseName string, actionConfig *action.Configuration, clientGetter *genericclioptions.ConfigFlags, settings *cli.EnvSettings, log logr.Logger) (error, bool) {
    defer wg.Done()
    chart, err := loader.Load(chartInstallation.Path)
    if err != nil {
        return err, true
    }
    releaseName = releaseName + "-" + chartInstallation.Name
    if err := actionConfig.Init(clientGetter, settings.Namespace(), os.Getenv("HELM_DRIVER"), func(format string, v ...interface{}) {
        r := fmt.Sprintf(format, v)
        log.Info("Helm Installation", "deploy status", r)
    }); err != nil {
        return err, true
    }
    releasePresent := true
    status, err := action.NewStatus(actionConfig).Run(releaseName)
    if err != nil {
        //if errors.Is(err, driver.ErrReleaseNotFound) {
        if strings.Contains(err.Error(), driver.ErrReleaseNotFound.Error()) {
            releasePresent = false
        } else {
            return err, true
        }
    }

    if !releasePresent {
        // install chart
        installAction := action.NewInstall(actionConfig)
        installAction.CreateNamespace = true
        installAction.Namespace = chartInstallation.Namespace
        installAction.ReleaseName = releaseName
        
        installAction.Wait = true
        // install new
        _, err := installAction.Run(chart, nil)
        if err != nil {
            return err, true
        }
        log.Info("installed: ", "releaseName", releaseName)
    }

    if status != nil {
        if releasePresent && status.Info.Status.String() == release.StatusFailed.String() { // upgrade if broken
            upgradeAction := action.NewUpgrade(actionConfig)
            upgradeAction.Atomic = false
            upgradeAction.CleanupOnFail = true
            upgradeAction.Wait = true
            upgradeAction.ReuseValues = false
            upgradeAction.Recreate = false
            _, err := upgradeAction.Run(releaseName, chart, nil)
            if err != nil {
                return err, true
            }
        }
    }
    return nil, false
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$
  1. The code is ignoring the return values from the go routine
  2. The WorkGroup isn't used to block until completion
// 1. Perhaps you'd want to have a separate channel to communicate the errors.
var errCh := make(chan error, len(charts))
for _, chartInstallation := range charts {
    wg.Add(1)
    go func(errCh chan error){
       installed, err := installChart(&wg,chartInstallation, releaseName, actionConfig, clientGetter, settings, log)
       if err != nil {
           errCh <- err
       }
    }(errCh)
}
// 2. Wait for the wait group to complete
wg.Wait()
// 1. Collect the errors
for err := range errCh {
    fmt.Println("Error: ", err)
}
\$\endgroup\$
3
  • \$\begingroup\$ Thanks!1+ , should I use the errCh or I can just use the log in if err != nil {log.error(...) , not sure I understand what are the benefit of the channel here, could you please explain? \$\endgroup\$ Commented Apr 19, 2021 at 10:28
  • \$\begingroup\$ The errCh is intended to communicate errors to the main goroutine so it could handle them if necessary. If all you care is to output the log IMO it wouldn't matter from which goroutine you are printing. But there are cases where you would like to cancel all routines for a given error. i.e. if the error is terminal, cancel all other goroutines, collect the errors and handle them as needed and then exit. \$\endgroup\$ Commented Apr 19, 2021 at 12:07
  • 1
    \$\begingroup\$ Welcome to the Code Review Community. Good answer, +1. Just a suggestion for the future, instead of You are ignoring it would be better if it was The code is ignoring. The use of you or your could make it seem like a personal attack. \$\endgroup\$ Commented Apr 19, 2021 at 14:04

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.