Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(helm): pass --debug option to helm
This week I stuck with a pretty ugly YAML formatting issue that took me quite some time to figure out what was the problem about. During troubleshooting I was using `werf render with --debug`, but the error message (something like `error converting YAML to JSON`) was not the one that clearly says what should be done to fix the issue.. It was not an issue with `werf` at all, it is more about human factor :)) So, I did some research regarding YAML troubleshooting and `helm template --debug` comes to play. It was the thing that in my opition makes the troubleshooting process a way simpler, because it not just returnes errors, but shows the YAML dump, so you can clearly see what is wrong with templating. Next, I found that during `werf helm template --debug`, the debug flag was not passed to the `helm`. At the same time `HELM_DEBUG` worked fine. This PR allows to use `werf helm template --debug` as expected (BTW, originally `--debug` is a global param for all helm subcommands). Nevertheless, this PR works fine for me, I would like to highlight a cople of moments: 1) Currently, `WERF_DEBUG` and `WERF_LOG_DEBUG` variables can enable helm logging as well as `HELM_DEBUG`. The following `cmd/werf/common/common.go` function should be refactored to change the behaviour: ``` func setupLogDebug(cmdData *CmdData, cmd *cobra.Command) { cmdData.LogDebug = new(bool) defaultValue := false for _, envName := range []string{ "WERF_LOG_DEBUG", "WERF_DEBUG", } { if os.Getenv(envName) != "" { defaultValue = util.GetBoolEnvironmentDefaultFalse(envName) break } } for alias, env := range map[string]string{ "log-debug": "WERF_LOG_DEBUG", "debug": "WERF_DEBUG", } { cmd.PersistentFlags().BoolVarP( cmdData.LogDebug, alias, "", defaultValue, fmt.Sprintf("Enable debug (default $%s).", env), ) } if err := cmd.PersistentFlags().MarkHidden("debug"); err != nil { panic(err) } } I believe we have a good reason to panic in our code. If we touch this part, we also should think about the following code in the `cmd/werf/helm/helm.go`: ``` if err := common.ProcessLogOptions(&_commonCmdData); err != nil { common.PrintHelp(cmd) return err } ``` I see some references to log formatting and color effects, but I am not sure if we really need this part in the case of calling an external plugin like helm. 2) Currently helm `--debug` output has additional info, that is not present in originall helm binary. See a sample below: ``` werf helm template .helm --debug 76.872875ms GetServiceValues result (err=%!s(<nil>)): 76.876541ms global: 76.877791ms env: "" 76.878916ms werf: 76.879958ms name: PROJECT 76.881458ms version: dev 76.882875ms werf: 76.884083ms commit: 76.885208ms date: 76.88625ms human: 1970-01-01 03:00:00 +0300 MSK 76.904ms unix: 0 76.920916ms hash: COMMIT_HASH 76.927666ms env: "" 76.936625ms image: {} 76.943416ms is_stub: true 76.973ms name: PROJECT 76.974583ms repo: REPO 76.975916ms stub_image: REPO:TAG 76.977708ms tag: {} 76.978875ms version: dev 76.984ms 77.829708ms install.go:253: [debug] Original chart version: "" 77.870708ms install.go:282: [debug] CHART PATH: /Users/idrey/code/werf-test/.helm 77.877791ms --- apiVersion: apps/v1 kind: Deployment <normal helm debug output skipped> ``` Actually, I do found additional output useful, but someone can found this extra section unnecessary. On the other hand under the hood during calling helm we already do a lot of additonal staff, so `helm` called from `werf` should not pretend to look like original `helm` )) Finally, let me know if you want me to refactor the code on above. Regards, Ilya Signed-off-by: Ilya Drey <ilya.drey@flant.com>
- Loading branch information