Prefer environment variables to CLI flags

Signed-off-by: Evans Mungai <mbuevans@gmail.com>

cmd/helm/profiling.go

@@ -19,30 +19,29 @@ package main
 import (
 	"fmt"
 	"os"
-	"path"
-	"path/filepath"
 	"runtime"
 	"runtime/pprof"
 	"strings"
-
-	"github.com/spf13/cobra"
 )
 
 var (
 	cpuProfileFile *os.File
-	pprofPaths     map[string]string
+	cpuProfilePath string
+	memProfilePath string
 )
 
 func init() {
-	pprofPaths = parsePProfPaths(os.Getenv("HELM_PPROF"))
+	cpuProfilePath = os.Getenv("HELM_PPROF_CPU_PROFILE")
+	memProfilePath = os.Getenv("HELM_PPROF_MEM_PROFILE")
 }
 
-// startProfiling starts profiling CPU usage
+// startProfiling starts profiling CPU usage if HELM_PPROF_CPU_PROFILE is set
+// to a file path. It returns an error if the file could not be created or
+// CPU profiling could not be started.
 func startProfiling() error {
-	cpuprofile, ok := pprofPaths["cpu"]
-	if ok && cpuprofile != "" {
+	if cpuProfilePath != "" {
 		var err error
-		cpuProfileFile, err = os.Create(cpuprofile)
+		cpuProfileFile, err = os.Create(cpuProfilePath)
 		if err != nil {
 			return fmt.Errorf("could not create CPU profile: %w", err)
 		}
@@ -55,8 +54,9 @@ func startProfiling() error {
 	return nil
 }
 
-// stopProfiling stops profiling CPU and memory usage and writes the results to
-// the files specified by HELM_PPROF=cpu=/path/to/cpu.prof,mem=/path/to/mem.prof
+// stopProfiling stops profiling CPU and memory usage.
+// It writes memory profile to the file path specified in HELM_PPROF_MEM_PROFILE
+// environment variable.
 func stopProfiling() error {
 	errs := []string{}
 
@@ -70,9 +70,8 @@ func stopProfiling() error {
 		cpuProfileFile = nil
 	}
 
-	memprofile, ok := pprofPaths["mem"]
-	if ok && memprofile != "" {
-		f, err := os.Create(memprofile)
+	if memProfilePath != "" {
+		f, err := os.Create(memProfilePath)
 		if err != nil {
 			errs = append(errs, err.Error())
 		}
@@ -90,35 +89,3 @@ func stopProfiling() error {
 
 	return nil
 }
-
-// addProfilingFlags adds the --cpuprofile and --memprofile flags to the given command.
-func addProfilingFlags(cmd *cobra.Command) {
-	// Persistent flags to make available to subcommands
-	cmd.PersistentFlags().String("cpuprofile", "", "File path to write cpu profiling data")
-	cmd.PersistentFlags().String("memprofile", "", "File path to write memory profiling data")
-}
-
-func parsePProfPaths(env string) map[string]string {
-	// Initial empty paths
-	m := map[string]string{}
-	for _, pprofs := range strings.Split(env, ",") {
-		// Is of the format mem=/path/to/memprof
-		tuple := strings.Split(pprofs, "=")
-		if len(tuple) != 2 {
-			continue
-		}
-		if tuple[0] != "cpu" && tuple[0] != "mem" {
-			continue
-		}
-
-		s, err := filepath.Abs(path.Clean(tuple[1]))
-		if err != nil {
-			continue
-		}
-		if !strings.HasSuffix(s, string(filepath.Separator)) {
-			// Ensure its not a directory
-			m[tuple[0]] = s
-		}
-	}
-	return m
-}

cmd/helm/profiling_test.go

@@ -1,79 +0,0 @@
-/*
-Copyright The Helm Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package main
-
-import (
-	"os"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-func Test_parsePProfPaths(t *testing.T) {
-	cwd, err := os.Getwd()
-	require.NoError(t, err)
-
-	tests := []struct {
-		name string
-		env  string
-		want map[string]string
-	}{
-		{
-			name: "no env",
-			env:  "",
-			want: map[string]string{},
-		},
-		{
-			name: "single path",
-			env:  "cpu=cpu.pprof",
-			want: map[string]string{
-				"cpu": cwd + "/cpu.pprof",
-			},
-		},
-		{
-			name: "mem and cpu paths",
-			env:  "cpu=cpu.pprof,mem=mem.pprof",
-			want: map[string]string{
-				"cpu": cwd + "/cpu.pprof",
-				"mem": cwd + "/mem.pprof",
-			},
-		},
-		{
-			name: "extra commas",
-			env:  "cpu=cpu.pprof,mem=mem.pprof,",
-			want: map[string]string{
-				"cpu": cwd + "/cpu.pprof",
-				"mem": cwd + "/mem.pprof",
-			},
-		},
-		{
-			name: "unknown keys",
-			env:  "cpu=cpu.pprof,mem=mem.pprof,foo=bar",
-			want: map[string]string{
-				"cpu": cwd + "/cpu.pprof",
-				"mem": cwd + "/mem.pprof",
-			},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got := parsePProfPaths(tt.env)
-			assert.Equalf(t, tt.want, got, "parsePProfPaths() = %v, want %v", got, tt.want)
-		})
-	}
-}

cmd/helm/root.go

@@ -216,9 +216,6 @@ func newRootCmd(actionConfig *action.Configuration, out io.Writer, args []string
 	// Check for expired repositories
 	checkForExpiredRepos(settings.RepositoryConfig)
 
-	// CPU and memory profiling flags that are available to all commands
-	addProfilingFlags(cmd)
-
 	return cmd, nil
 }