From b87674f1203241ed06cc9e211ef19dd1f09c6b16 Mon Sep 17 00:00:00 2001 From: chaozbj Date: Fri, 17 Jul 2020 15:04:41 +0800 Subject: [PATCH 1/5] add profiling subcommand Signed-off-by: chaozbj --- plugins/admin/core/root.go | 2 + .../admin/pkg/command/profiling/profiling.go | 183 ++++++++++++++++++ 2 files changed, 185 insertions(+) create mode 100644 plugins/admin/pkg/command/profiling/profiling.go diff --git a/plugins/admin/core/root.go b/plugins/admin/core/root.go index 013b9dfc..9166fad2 100644 --- a/plugins/admin/core/root.go +++ b/plugins/admin/core/root.go @@ -25,6 +25,7 @@ import ( "knative.dev/client-contrib/plugins/admin/pkg/command" "knative.dev/client-contrib/plugins/admin/pkg/command/autoscaling" "knative.dev/client-contrib/plugins/admin/pkg/command/domain" + "knative.dev/client-contrib/plugins/admin/pkg/command/profiling" private_registry "knative.dev/client-contrib/plugins/admin/pkg/command/registry" ) @@ -50,6 +51,7 @@ func NewAdminCommand(params ...pkg.AdminParams) *cobra.Command { rootCmd.AddCommand(domain.NewDomainCmd(p)) rootCmd.AddCommand(private_registry.NewPrivateRegistryCmd(p)) rootCmd.AddCommand(autoscaling.NewAutoscalingCmd(p)) + rootCmd.AddCommand(profiling.NewProfilingCommand(p)) rootCmd.AddCommand(command.NewVersionCommand()) // Add default help page if there's unknown command diff --git a/plugins/admin/pkg/command/profiling/profiling.go b/plugins/admin/pkg/command/profiling/profiling.go new file mode 100644 index 00000000..fdf9b7db --- /dev/null +++ b/plugins/admin/pkg/command/profiling/profiling.go @@ -0,0 +1,183 @@ +// Copyright © 2020 The Knative 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 profiling + +import ( + //"encoding/json" + //"errors" + "fmt" + "strconv" + "strings" + //"sync" + + "github.com/spf13/cobra" + + corev1 "k8s.io/api/core/v1" + //apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + //"k8s.io/apimachinery/pkg/labels" + "k8s.io/client-go/kubernetes" + "knative.dev/client-contrib/plugins/admin/pkg" + "knative.dev/client-contrib/plugins/admin/pkg/command/utils" +) + +const ( + profilingExample = ` + # To enable profiling + kn admin profiling --enable + + # To download heap profile data of autoscaler + kn admin profiling --target autoscaler --heap + + # To download 2 minutes execution trace data of networking-istio + kn admin profiling --target networking-istio --trace 2m + + # To download cpu profile data of activator and save to specified folder + kn admin profiling --target activator --profile --save-path /tmp + + # To download all available profile data for specified pod activator-5979f56548 + kn admin profiling --target activator-5979f56548 --all +` + + targetFlagUsgae = "The profiling target. It can be a Knative component name or a specific pod name, e.g: activator or activator-5979f56548" + savePathFlagUsage = "The path to save the downloaded profile data, if not speicifed, the data will be saved in current working folder" + heapFlagUsage = "Download heap profile data" + cpuFlagUsage = "Download cpu profile data, you can specify the duration with seconds, minutes or hours, e.g: 1m for one minute cpu profile data" + blockFlagUsgae = "Download Go routine blocking data" + traceFlagUsage = "Download execution trace data, you can specify the duration with seconds, minutes or hours, e.g: 5s for 5 seconds trace data" + memoryAllocsFlagUsage = "Download all memory allocations data" + mutexFlagUsage = "Download holders of contended mutexes data" + goroutineFlagUsage = "Download stack traces of all current goroutines" + threadCreateFlagUsage = "Download stack traces that led to the creation of new OS threads" + + knNamespace = "knative-serving" + obsConfigMap = "config-observability" +) + +// NewProfilingCommand creates a profiling command +func NewProfilingCommand(p *pkg.AdminParams) *cobra.Command { + var enableProfiling bool + var disableProfiling bool + var target string + var savePath string + //var heapFlagVal bool + var cpuFlagVal string + //var blockFlagVal bool + var traceFlagVal string + //var memAllocsFlagVal bool + //var goroutineFlagVal bool + //var threadCreateFlagVal bool + + var profilingCmd = &cobra.Command{ + Use: "profiling", + Aliases: []string{"prof"}, + Short: "Profiling Knative components", + Long: `Enable Knative components profiling and download profile data`, + Example: profilingExample, + PreRunE: func(cmd *cobra.Command, args []string) error { + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + if cmd.Flags().Changed("enable") { + return configProfiling(p.ClientSet, cmd, true) + } else if cmd.Flags().Changed("disable") { + return configProfiling(p.ClientSet, cmd, false) + } + return nil + }, + } + + flags := profilingCmd.Flags() + flags.BoolVar(&enableProfiling, "enable", false, "Enable Knative profiling") + flags.BoolVar(&disableProfiling, "disable", false, "Disable Knative profiling") + flags.StringVarP(&target, "target", "t", "", targetFlagUsgae) + flags.StringVarP(&savePath, "save-path", "s", "", savePathFlagUsage) + flags.Bool("heap", false, heapFlagUsage) + flags.StringVar(&cpuFlagVal, "cpu", "30s", cpuFlagUsage) + flags.Bool("block", false, blockFlagUsgae) + flags.StringVar(&traceFlagVal, "trace", "30s", traceFlagUsage) + flags.Bool("mem-allocs", false, memoryAllocsFlagUsage) + flags.Bool("mutex", false, mutexFlagUsage) + flags.Bool("goroutine", false, goroutineFlagUsage) + flags.Bool("thread-create", false, threadCreateFlagUsage) + return profilingCmd +} + +func configProfiling(c kubernetes.Interface, cmd *cobra.Command, enable bool) error { + currentCm := &corev1.ConfigMap{} + currentCm, err := c.CoreV1().ConfigMaps(knNamespace).Get(obsConfigMap, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("failed to get ConfigMap %s in namespace %s: %+v", obsConfigMap, knNamespace, err) + } + + desiredCm := currentCm.DeepCopy() + if enable { + desiredCm.Data["profiling.enable"] = "true" + } else { + desiredCm.Data["profiling.enable"] = "false" + } + + //cmd.Printf("%+v", *desiredCm) + //cmd.Printf("Enable: %+v", currentCm.Data["profiling.enable"]) + err = utils.UpdateConfigMap(c, desiredCm) + if err != nil { + return fmt.Errorf("failed to update ConfigMap %s in namespace %s: %+v", obsConfigMap, knNamespace, err) + } + + if enable { + cmd.Print("Knative profiling is enabled") + } else { + cmd.Print("Knative profiling is disabled") + } + return nil +} + +func isProfilingEnabled(c kubernetes.Interface) (bool, error) { + currentCm := &corev1.ConfigMap{} + currentCm, err := c.CoreV1().ConfigMaps(knNamespace).Get(obsConfigMap, metav1.GetOptions{}) + if err != nil { + return false, fmt.Errorf("failed to get ConfigMap %s in namespace %s: %+v", obsConfigMap, knNamespace, err) + } + + if strings.ToLower(currentCm.Data["profiling.enable"]) == "true" { + return true, nil + } + return false, nil +} + +// parseDuration parses the given duration string to integer seconds, the duration is an integer plusing a character +// 's', 'm' and 'h' to express seconds, minutes and hours +func parseDuration(duration string) (int, error) { + l := len(duration) + if l < 2 { + return 0, fmt.Errorf("invalid duation: %s", duration) + } + + unit := strings.ToLower(duration[l-1:]) + n, err := strconv.ParseInt(duration[:l-1], 10, 32) + if err != nil { + return 0, err + } + + if unit == "s" { + return int(n), nil + } else if unit == "m" { + return int(n) * 60, nil + } else if unit == "h" { + return int(n) * 3600, nil + } else { + return 0, fmt.Errorf("invalid duration: %s, only supports 's', 'm' and 'h' units", duration) + } +} From a823f056019b359aa3b51165da6023aeebd67cab Mon Sep 17 00:00:00 2001 From: chaozbj Date: Sat, 25 Jul 2020 17:03:18 +0800 Subject: [PATCH 2/5] implemented profiling command and ut codes Signed-off-by: chaozbj --- plugins/admin/README.adoc | 72 +++ plugins/admin/core/root_test.go | 1 + .../admin/pkg/command/profiling/profiling.go | 386 ++++++++++-- .../pkg/command/profiling/profiling_test.go | 560 ++++++++++++++++++ 4 files changed, 966 insertions(+), 53 deletions(-) create mode 100644 plugins/admin/pkg/command/profiling/profiling_test.go diff --git a/plugins/admin/README.adoc b/plugins/admin/README.adoc index ee8834c6..07c3d5ae 100644 --- a/plugins/admin/README.adoc +++ b/plugins/admin/README.adoc @@ -114,6 +114,56 @@ Global Flags: Use "kn admin autoscaling [command] --help" for more information about a command. +---- + +#### `kn admin profiling` + +---- +Enable Knative components profiling and download profile data + +Usage: + kn admin profiling [flags] + +Aliases: + profiling, prof + +Examples: + + # To enable profiling + kn admin profiling --enable + + # To download heap profile data of autoscaler + kn admin profiling --target autoscaler --heap + + # To download 2 minutes execution trace data of networking-istio + kn admin profiling --target networking-istio --trace 2m + + # To download go routing block and memory allocations data of activator and save them to /tmp + kn admin profiling --target activator --block --mem-allocs --save-to /tmp + + # To download all available profile data for specified pod activator-5979f56548 + kn admin profiling --target activator-5979f56548 --all + + +Flags: + --all Download all profile data + --block Download go routine blocking data + --cpu string Download cpu profile data, you can specify a profile data duration with 's' for second(s), 'm' for minute(s) and 'h' for hour(s), e.g: '1m' for one minute (default "5") + --disable Disable Knative profiling + --enable Enable Knative profiling + --goroutine Download stack traces of all current goroutines data + --heap Download heap profile data + -h, --help help for profiling + --mem-allocs Download memory allocations data + --mutex Download holders of contended mutexes data + -s, --save-to string The path to save the downloaded profile data, if not speicifed, the data will be saved in current working folder + -t, --target string The profiling target. It can be a Knative component name or a specific pod name, e.g: 'activator' or 'activator-586d468c99-w59cm' + --thread-create Download stack traces that led to the creation of new OS threads data + --trace string Download execution trace data, you can specify a trace data duration with 's' for second(s), 'm' for minute(s) and 'h' for hour(s), e.g: '1m' for one minute (default "5") + +Global Flags: + --config string config file (default is $HOME/.config/kn/plugins/admin.yaml) + ---- ### Examples @@ -174,3 +224,25 @@ $ kn admin autoscaling update --scale-to-zero Updated Knative autoscaling config enable-scale-to-zero: true ----- ===== + +#### As a Knative administrator, I want to enable Knative profiling and download profile data. + +.Enable Knative profiling. +===== +----- +$ kn admin profiling --enable +Knative profiling is enabled +----- +===== + +.Download 5 seconds cpu profile data of activator component and save data to /tmp folder +===== +----- +$ kn admin profiling --target activator --cpu 5s --save-to /tmp +Starting to download profile data for pod activator-586d468c99-w59cm... +Saving 5 second(s) cpu profile data to /tmp/activator-586d468c99-w59cm_cpu_5s_20200725165758 +Forwarding from 127.0.0.1:18008 -> 8008 +Forwarding from [::1]:18008 -> 8008 +Handling connection for 18008 +----- +===== \ No newline at end of file diff --git a/plugins/admin/core/root_test.go b/plugins/admin/core/root_test.go index f6d0a4d6..3278fb3a 100644 --- a/plugins/admin/core/root_test.go +++ b/plugins/admin/core/root_test.go @@ -33,6 +33,7 @@ func TestNewAdminCommand(t *testing.T) { "domain", "registry", "autoscaling", + "profiling", } cmd := NewAdminCommand() diff --git a/plugins/admin/pkg/command/profiling/profiling.go b/plugins/admin/pkg/command/profiling/profiling.go index fdf9b7db..6e084d13 100644 --- a/plugins/admin/pkg/command/profiling/profiling.go +++ b/plugins/admin/pkg/command/profiling/profiling.go @@ -15,19 +15,17 @@ package profiling import ( - //"encoding/json" - //"errors" "fmt" + "os" + "path/filepath" "strconv" "strings" - //"sync" + "time" "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" - //apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - //"k8s.io/apimachinery/pkg/labels" "k8s.io/client-go/kubernetes" "knative.dev/client-contrib/plugins/admin/pkg" "knative.dev/client-contrib/plugins/admin/pkg/command/utils" @@ -44,41 +42,68 @@ const ( # To download 2 minutes execution trace data of networking-istio kn admin profiling --target networking-istio --trace 2m - # To download cpu profile data of activator and save to specified folder - kn admin profiling --target activator --profile --save-path /tmp + # To download go routing block and memory allocations data of activator and save them to /tmp + kn admin profiling --target activator --block --mem-allocs --save-to /tmp # To download all available profile data for specified pod activator-5979f56548 kn admin profiling --target activator-5979f56548 --all ` - targetFlagUsgae = "The profiling target. It can be a Knative component name or a specific pod name, e.g: activator or activator-5979f56548" - savePathFlagUsage = "The path to save the downloaded profile data, if not speicifed, the data will be saved in current working folder" + targetFlagUsgae = "The profiling target. It can be a Knative component name or a specific pod name, e.g: 'activator' or 'activator-586d468c99-w59cm'" + saveToFlagUsage = "The path to save the downloaded profile data, if not speicifed, the data will be saved in current working folder" + allFlagUsage = "Download all available profile data" + cpuFlagUsage = "Download cpu profile data, you can specify a profile data duration with 's' for second(s), 'm' for minute(s) and 'h' for hour(s), e.g: '1m' for one minute" heapFlagUsage = "Download heap profile data" - cpuFlagUsage = "Download cpu profile data, you can specify the duration with seconds, minutes or hours, e.g: 1m for one minute cpu profile data" - blockFlagUsgae = "Download Go routine blocking data" - traceFlagUsage = "Download execution trace data, you can specify the duration with seconds, minutes or hours, e.g: 5s for 5 seconds trace data" - memoryAllocsFlagUsage = "Download all memory allocations data" + blockFlagUsage = "Download go routine blocking data" + traceFlagUsage = "Download execution trace data, you can specify a trace data duration with 's' for second(s), 'm' for minute(s) and 'h' for hour(s), e.g: '1m' for one minute" + memAllocsFlagUsage = "Download memory allocations data" mutexFlagUsage = "Download holders of contended mutexes data" - goroutineFlagUsage = "Download stack traces of all current goroutines" - threadCreateFlagUsage = "Download stack traces that led to the creation of new OS threads" + goroutineFlagUsage = "Download stack traces of all current goroutines data" + threadCreateFlagUsage = "Download stack traces that led to the creation of new OS threads data" - knNamespace = "knative-serving" - obsConfigMap = "config-observability" + cpuFlagName = "cpu" + heapFlagName = "heap" + blockFlagName = "block" + traceFlagName = "trace" + memAllocsFlagName = "mem-allocs" + mutexFlagName = "mutex" + goroutineFlagName = "goroutine" + threadCreateFlagName = "thread-create" + knNamespace = "knative-serving" + obsConfigMap = "config-observability" + defaultDuration = 5 + defaultProfilingTime = ProfilingTime(defaultDuration * time.Second) ) +// profilingFlags defines flag values for profiling command +type profilingFlags struct { + enable bool + disable bool + target string + saveTo string + allProfiles bool + cpuProfile string + heapProfile bool + blockProfile bool + traceProfile string + memAllocsProfile bool + mutexProfile bool + goroutineProfile bool + threadCreateProfile bool +} + +// profileTypeOption is a helper struct to download profile type data +type profileTypeOption struct { + profileType ProfileType + downloadOption DownloadOptions +} + +// declare newDownloaderFunc variable to help us write UT +var newDownloaderFunc = NewDownloader + // NewProfilingCommand creates a profiling command func NewProfilingCommand(p *pkg.AdminParams) *cobra.Command { - var enableProfiling bool - var disableProfiling bool - var target string - var savePath string - //var heapFlagVal bool - var cpuFlagVal string - //var blockFlagVal bool - var traceFlagVal string - //var memAllocsFlagVal bool - //var goroutineFlagVal bool - //var threadCreateFlagVal bool + pflags := profilingFlags{} var profilingCmd = &cobra.Command{ Use: "profiling", @@ -87,34 +112,75 @@ func NewProfilingCommand(p *pkg.AdminParams) *cobra.Command { Long: `Enable Knative components profiling and download profile data`, Example: profilingExample, PreRunE: func(cmd *cobra.Command, args []string) error { + flags := cmd.Flags() + // no flag given, print help + if flags.NFlag() < 1 { + cmd.Help() + return nil + } + + isEnableSet := flags.Changed("enable") + isDisableSet := flags.Changed("disable") + isTargetSet := flags.Changed("target") + isSaveToSet := flags.Changed("save-to") + isAllProfilesSet := flags.Changed("all") + isProfileTypeSet := (flags.Changed(cpuFlagName) || flags.Changed(heapFlagName) || flags.Changed(blockFlagName) || + flags.Changed(traceFlagName) || flags.Changed(memAllocsFlagName) || flags.Changed(mutexFlagName) || + flags.Changed(goroutineFlagName) || flags.Changed(threadCreateFlagName)) + + // enable and disable can't be used togerther + if isEnableSet && isDisableSet { + return fmt.Errorf("flags '--enable' and '--disable' can not be used together") + } + + // enable or disable can't be used with other flags + if (isEnableSet || isDisableSet) && (isTargetSet || isProfileTypeSet || isAllProfilesSet || isSaveToSet) { + return fmt.Errorf("flag '--enable' or '--disable' can not be used with other flags") + } + + // --target flag is needed + if !isTargetSet && (isProfileTypeSet || isAllProfilesSet || isSaveToSet) { + return fmt.Errorf("requires '--target' flag") + } + + // --profile-type is needed + if !isProfileTypeSet && !isAllProfilesSet && (isTargetSet || isSaveToSet) { + return fmt.Errorf("requires '--all' or a specific profile type flag") + } return nil }, RunE: func(cmd *cobra.Command, args []string) error { - if cmd.Flags().Changed("enable") { + flags := cmd.Flags() + if flags.NFlag() < 1 { + return nil + } else if flags.Changed("enable") { return configProfiling(p.ClientSet, cmd, true) - } else if cmd.Flags().Changed("disable") { + } else if flags.Changed("disable") { return configProfiling(p.ClientSet, cmd, false) + } else { + return downloadProfileData(p, cmd, &pflags) } - return nil }, } flags := profilingCmd.Flags() - flags.BoolVar(&enableProfiling, "enable", false, "Enable Knative profiling") - flags.BoolVar(&disableProfiling, "disable", false, "Disable Knative profiling") - flags.StringVarP(&target, "target", "t", "", targetFlagUsgae) - flags.StringVarP(&savePath, "save-path", "s", "", savePathFlagUsage) - flags.Bool("heap", false, heapFlagUsage) - flags.StringVar(&cpuFlagVal, "cpu", "30s", cpuFlagUsage) - flags.Bool("block", false, blockFlagUsgae) - flags.StringVar(&traceFlagVal, "trace", "30s", traceFlagUsage) - flags.Bool("mem-allocs", false, memoryAllocsFlagUsage) - flags.Bool("mutex", false, mutexFlagUsage) - flags.Bool("goroutine", false, goroutineFlagUsage) - flags.Bool("thread-create", false, threadCreateFlagUsage) + flags.BoolVar(&pflags.enable, "enable", false, "Enable Knative profiling") + flags.BoolVar(&pflags.disable, "disable", false, "Disable Knative profiling") + flags.StringVarP(&pflags.target, "target", "t", "", targetFlagUsgae) + flags.StringVarP(&pflags.saveTo, "save-to", "s", "", saveToFlagUsage) + flags.BoolVar(&pflags.allProfiles, "all", false, allFlagUsage) + flags.StringVarP(&pflags.cpuProfile, cpuFlagName, "", "5", cpuFlagUsage) + flags.BoolVar(&pflags.heapProfile, heapFlagName, false, heapFlagUsage) + flags.BoolVar(&pflags.blockProfile, blockFlagName, false, blockFlagUsage) + flags.StringVarP(&pflags.traceProfile, traceFlagName, "", "5", traceFlagUsage) + flags.BoolVar(&pflags.memAllocsProfile, memAllocsFlagName, false, memAllocsFlagUsage) + flags.BoolVar(&pflags.mutexProfile, mutexFlagName, false, mutexFlagUsage) + flags.BoolVar(&pflags.goroutineProfile, goroutineFlagName, false, goroutineFlagUsage) + flags.BoolVar(&pflags.threadCreateProfile, threadCreateFlagName, false, threadCreateFlagUsage) return profilingCmd } +// configProfiling enables or disables knative profiling func configProfiling(c kubernetes.Interface, cmd *cobra.Command, enable bool) error { currentCm := &corev1.ConfigMap{} currentCm, err := c.CoreV1().ConfigMaps(knNamespace).Get(obsConfigMap, metav1.GetOptions{}) @@ -129,21 +195,20 @@ func configProfiling(c kubernetes.Interface, cmd *cobra.Command, enable bool) er desiredCm.Data["profiling.enable"] = "false" } - //cmd.Printf("%+v", *desiredCm) - //cmd.Printf("Enable: %+v", currentCm.Data["profiling.enable"]) err = utils.UpdateConfigMap(c, desiredCm) if err != nil { return fmt.Errorf("failed to update ConfigMap %s in namespace %s: %+v", obsConfigMap, knNamespace, err) } if enable { - cmd.Print("Knative profiling is enabled") + cmd.Println("Knative profiling is enabled") } else { - cmd.Print("Knative profiling is disabled") + cmd.Println("Knative profiling is disabled") } return nil } +// isProfilingEnabled checks if the profiling is enabled func isProfilingEnabled(c kubernetes.Interface) (bool, error) { currentCm := &corev1.ConfigMap{} currentCm, err := c.CoreV1().ConfigMaps(knNamespace).Get(obsConfigMap, metav1.GetOptions{}) @@ -160,24 +225,239 @@ func isProfilingEnabled(c kubernetes.Interface) (bool, error) { // parseDuration parses the given duration string to integer seconds, the duration is an integer plusing a character // 's', 'm' and 'h' to express seconds, minutes and hours func parseDuration(duration string) (int, error) { + duration = strings.TrimSpace(duration) + if duration == "" { + return defaultDuration, nil + } + l := len(duration) - if l < 2 { - return 0, fmt.Errorf("invalid duation: %s", duration) + unit := duration[l-1] + // duration is numberic + if unit >= '0' && unit <= '9' { + n, err := strconv.ParseInt(duration, 10, 32) + if err != nil { + return 0, err + } + return int(n), nil } - unit := strings.ToLower(duration[l-1:]) + // parse duration[:l-1] as int n, err := strconv.ParseInt(duration[:l-1], 10, 32) if err != nil { return 0, err } - if unit == "s" { + // calculate duration by unit + if unit == 's' || unit == 'S' { return int(n), nil - } else if unit == "m" { + } else if unit == 'm' || unit == 'M' { return int(n) * 60, nil - } else if unit == "h" { + } else if unit == 'h' || unit == 'H' { return int(n) * 3600, nil } else { return 0, fmt.Errorf("invalid duration: %s, only supports 's', 'm' and 'h' units", duration) } } + +// durationDescription describes a given seconds as 0h0m0s format +func durationDescription(seconds int64) string { + if seconds < 1 { + return fmt.Sprintf("%ds", seconds) + } + + s := "" + if hours := seconds / 3600; hours > 0 { + s = fmt.Sprintf("%dh", hours) + } + left := seconds % 3600 + if mins := left / 60; mins > 0 { + s = fmt.Sprintf("%s%dm", s, mins) + } + if secs := left % 60; secs > 0 { + s = fmt.Sprintf("%s%ds", s, secs) + } + return s +} + +// downloadProfileData downloads profile data by given profile type +func downloadProfileData(p *pkg.AdminParams, cmd *cobra.Command, pflags *profilingFlags) error { + // check profile types + profileTypes := map[string]profileTypeOption{} + if pflags.allProfiles { + // all profile types + profileTypes[cpuFlagName] = profileTypeOption{ + profileType: ProfileTypeProfile, + downloadOption: defaultProfilingTime, + } + profileTypes[heapFlagName] = profileTypeOption{profileType: ProfileTypeProfile} + profileTypes[blockFlagName] = profileTypeOption{profileType: ProfileTypeBlock} + profileTypes[traceFlagName] = profileTypeOption{ + profileType: ProfileTypeTrace, + downloadOption: defaultProfilingTime, + } + profileTypes[memAllocsFlagName] = profileTypeOption{profileType: ProfileTypeAllocs} + profileTypes[mutexFlagName] = profileTypeOption{profileType: ProfileTypeMutex} + profileTypes[goroutineFlagName] = profileTypeOption{profileType: ProfileTypeGoroutine} + profileTypes[threadCreateFlagName] = profileTypeOption{profileType: ProfileTypeThreadCreate} + } else { + flags := cmd.Flags() + // cpu profile type + if flags.Changed(cpuFlagName) { + op := profileTypeOption{profileType: ProfileTypeProfile} + if pflags.cpuProfile == "" { + op.downloadOption = defaultProfilingTime + } else { + duration, err := parseDuration(pflags.cpuProfile) + if err != nil { + return err + } + op.downloadOption = ProfilingTime(time.Duration(duration) * time.Second) + } + profileTypes[cpuFlagName] = op + } + // heap profile type + if flags.Changed(heapFlagName) { + profileTypes[heapFlagName] = profileTypeOption{profileType: ProfileTypeHeap} + } + // block profile type + if flags.Changed(blockFlagName) { + profileTypes[blockFlagName] = profileTypeOption{profileType: ProfileTypeBlock} + } + // trace profile type + if flags.Changed(traceFlagName) { + op := profileTypeOption{profileType: ProfileTypeTrace} + if pflags.traceProfile == "" { + op.downloadOption = defaultProfilingTime + } else { + duration, err := parseDuration(pflags.traceProfile) + if err != nil { + return err + } + op.downloadOption = ProfilingTime(time.Duration(duration) * time.Second) + } + profileTypes[traceFlagName] = op + } + // mem-allocs profile type + if flags.Changed(memAllocsFlagName) { + profileTypes[memAllocsFlagName] = profileTypeOption{profileType: ProfileTypeAllocs} + } + // mutex profile type + if flags.Changed(mutexFlagName) { + profileTypes[mutexFlagName] = profileTypeOption{profileType: ProfileTypeMutex} + } + // goroutine profile type + if flags.Changed(goroutineFlagName) { + profileTypes[goroutineFlagName] = profileTypeOption{profileType: ProfileTypeGoroutine} + } + // thread-create profile type + if flags.Changed(threadCreateFlagName) { + profileTypes[threadCreateFlagName] = profileTypeOption{profileType: ProfileTypeThreadCreate} + } + } + + // check --save-to path + var err error + if pflags.saveTo == "" { + pflags.saveTo, err = os.Getwd() + if err != nil { + return fmt.Errorf("fail to get current working folder to save profile data: %+v", err) + } + } else { + stat, err := os.Stat(pflags.saveTo) + if os.IsNotExist(err) { + return fmt.Errorf("the specified save path '%s' doesn't exist", pflags.saveTo) + } else if err != nil { + return err + } + if !stat.IsDir() { + return fmt.Errorf("the specified save path '%s' is not a folder", pflags.saveTo) + } + } + + // check if profiling is enabled, if not, print message to ask user enable it first + enabled, err := isProfilingEnabled(p.ClientSet) + if err != nil { + return err + } + if !enabled { + return fmt.Errorf("profiling is not enabled, please use '--enable' to enalbe it first") + } + + // try to find target as a knative component name + pods, err := p.ClientSet.CoreV1().Pods(knNamespace).List(metav1.ListOptions{LabelSelector: "app=" + pflags.target}) + if err != nil { + return err + } + // if no pod found, try to find target as a pod name in knative namespace + if len(pods.Items) < 1 { + pods, err = p.ClientSet.CoreV1().Pods(knNamespace).List(metav1.ListOptions{}) + if err != nil { + return err + } + if len(pods.Items) < 1 { + return fmt.Errorf("fail to get profiling target '%s'", pflags.target) + } + + // check if target is found as pod + found := false + for _, p := range pods.Items { + if p.Name == pflags.target { + pods.Items = []corev1.Pod{p} + found = true + break + } + } + if !found { + return fmt.Errorf("fail to get profiling target '%s'", pflags.target) + } + } + + restConfig, err := p.RestConfig() + if err != nil { + return err + } + + // iterates pods to download specified profile data + for _, pod := range pods.Items { + err = func() error { + cmd.Printf("Starting to download profile data for pod %s...\n", pod.Name) + end := make(chan struct{}) + downloader, err := newDownloaderFunc(restConfig, pod.Name, knNamespace, end) + if err != nil { + return err + } + defer close(end) + + // iterates specified profile types to download data + for k, v := range profileTypes { + duration := "" + filename := pod.Name + "_" + k + options := []DownloadOptions{} + if t, ok := v.downloadOption.(ProfilingTime); ok { + seconds := int64(time.Duration(t) / time.Second) + duration = strconv.FormatInt(seconds, 10) + " second(s) " + filename += "_" + durationDescription(seconds) + options = append(options, v.downloadOption) + } + filename += "_" + time.Now().Format("20060102150405") + dataFilePath := filepath.Join(pflags.saveTo, filename) + f, err := os.Create(dataFilePath) + if err != nil { + return err + } + + cmd.Printf("Saving %s%s profile data to %s\n", duration, k, dataFilePath) + err = downloader.Download(v.profileType, f, options...) + f.Close() + if err != nil { + return err + } + } + return nil + }() + if err != nil { + return err + } + } + return nil +} diff --git a/plugins/admin/pkg/command/profiling/profiling_test.go b/plugins/admin/pkg/command/profiling/profiling_test.go new file mode 100644 index 00000000..ae28f99e --- /dev/null +++ b/plugins/admin/pkg/command/profiling/profiling_test.go @@ -0,0 +1,560 @@ +// Copyright © 2020 The Knative 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 profiling + +import ( + "errors" + "fmt" + "io" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/spf13/cobra" + "gotest.tools/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8srt "k8s.io/apimachinery/pkg/runtime" + k8sfake "k8s.io/client-go/kubernetes/fake" + k8sfakecorev1 "k8s.io/client-go/kubernetes/typed/core/v1/fake" + "k8s.io/client-go/rest" + k8stesting "k8s.io/client-go/testing" + "knative.dev/client-contrib/plugins/admin/pkg" + + "knative.dev/client-contrib/plugins/admin/pkg/testutil" +) + +func newProfileCommand() *cobra.Command { + client := k8sfake.NewSimpleClientset(&corev1.ConfigMap{}) + p := pkg.AdminParams{ClientSet: client} + return NewProfilingCommand(&p) +} + +func newProfileCommandWith(cm *corev1.ConfigMap) (*cobra.Command, *k8sfake.Clientset) { + client := k8sfake.NewSimpleClientset(cm) + p := pkg.AdminParams{ClientSet: client} + return NewProfilingCommand(&p), client +} + +type fakeDownloader struct { + error error +} + +func (d *fakeDownloader) Download(t ProfileType, output io.Writer, options ...DownloadOptions) error { + return d.error +} + +func fakeDownloaderBuilder(newError, downloadError error) func(*rest.Config, string, string, <-chan struct{}) (ProfileDownloader, error) { + return func(*rest.Config, string, string, <-chan struct{}) (ProfileDownloader, error) { + return &fakeDownloader{error: downloadError}, newError + } +} + +func removeProfileDataFiles(nameFilter string) { + files, err := filepath.Glob(nameFilter) + if err == nil { + for _, f := range files { + os.Remove(f) + } + } +} + +// TestNewProfilingCommand tests the profiling command +func TestNewProfilingCommand(t *testing.T) { + t.Run("runs profiling without args", func(t *testing.T) { + out, err := testutil.ExecuteCommand(newProfileCommand(), "", "") + assert.NilError(t, err) + assert.Check(t, strings.Contains(out, " profiling [flags]"), "expected profiling help output") + }) + + t.Run("runs profiling with conflict args", func(t *testing.T) { + // --enable and --disable can't be used together + _, err := testutil.ExecuteCommand(newProfileCommand(), "--enable", "--disable") + assert.ErrorContains(t, err, "flags '--enable' and '--disable' can not be used together", err) + + // --enable or --disable --target can't be used with other flags + argsList := [][]string{ + {"--enable", "--target", "activator"}, + {"--disable", "--target", "activator"}, + {"--enable", "--save-to", "/tmp"}, + {"--disable", "--save-to", "/tmp"}, + {"--enable", "--all"}, + {"--disable", "--all"}, + {"--enable", "--cpu", "5"}, + {"--disable", "--cpu", "5"}, + {"--enable", "--heap"}, + {"--disable", "--heap"}, + {"--enable", "--block"}, + {"--disable", "--block"}, + {"--enable", "--trace", "1m"}, + {"--disable", "--trace", "1m"}, + {"--enable", "--mem-allocs"}, + {"--disable", "--mem-allocs"}, + {"--enable", "--mutex"}, + {"--disable", "--mutex"}, + {"--enable", "--goroutine"}, + {"--disable", "--goroutine"}, + {"--enable", "--thread-create"}, + {"--disable", "--thread-create"}, + } + for _, args := range argsList { + _, err := testutil.ExecuteCommand(newProfileCommand(), args...) + assert.ErrorContains(t, err, "flag '--enable' or '--disable' can not be used with other flags", err) + } + + // requires target + argsList = [][]string{ + {"--save-to", "/tmp"}, + {"--all"}, + {"--all", "--save-to", "/tmp"}, + {"--cpu", "5"}, + {"--cpu", "5", "--save-to", "/tmp"}, + {"--heap"}, + {"--heap", "--save-to", "/tmp"}, + {"--block"}, + {"--block", "--save-to", "/tmp"}, + {"--trace", "1m"}, + {"--trace", "1m", "--save-to", "/tmp"}, + {"--mem-allocs"}, + {"--mem-allocs", "--save-to", "/tmp"}, + {"--mutex"}, + {"--mutex", "--save-to", "/tmp"}, + {"--goroutine"}, + {"--goroutine", "--save-to", "/tmp"}, + {"--thread-create"}, + {"--thread-create", "--save-to", "/tmp"}, + } + for _, args := range argsList { + _, err := testutil.ExecuteCommand(newProfileCommand(), args...) + assert.ErrorContains(t, err, "requires '--target' flag", err) + } + + // requires profile type + argsList = [][]string{ + {"--target", "activator"}, + {"--target", "activator", "--save-to", "/tmp"}, + } + for _, args := range argsList { + _, err := testutil.ExecuteCommand(newProfileCommand(), args...) + assert.ErrorContains(t, err, "requires '--all' or a specific profile type flag", err) + } + }) + + t.Run("parses invalid duration", func(t *testing.T) { + // invalid numberic + _, err := parseDuration("1x2") + assert.ErrorContains(t, err, `parsing "1x2": invalid syntax`, err) + + // invalid unit + _, err = parseDuration("12x") + assert.ErrorContains(t, err, "invalid duration: 12x, only supports 's', 'm' and 'h' units", err) + + // not integer + _, err = parseDuration("1.2") + assert.ErrorContains(t, err, `parsing "1.2": invalid syntax`, err) + }) + + t.Run("parses valid duration", func(t *testing.T) { + // empty duration, return default 5 + n, err := parseDuration("") + assert.NilError(t, err) + assert.Equal(t, defaultDuration, n) + n, err = parseDuration(" ") + assert.NilError(t, err) + assert.Equal(t, defaultDuration, n) + + // duration is numberic + n, err = parseDuration("123") + assert.NilError(t, err) + assert.Equal(t, 123, n) + + // duration is seconds + n, err = parseDuration("20s") + assert.NilError(t, err) + assert.Equal(t, 20, n) + n, err = parseDuration("20S") + assert.NilError(t, err) + assert.Equal(t, 20, n) + + // duration is minutes + n, err = parseDuration("2m") + assert.NilError(t, err) + assert.Equal(t, 120, n) + n, err = parseDuration("2M") + assert.NilError(t, err) + assert.Equal(t, 120, n) + + // duration is hours + n, err = parseDuration("1h") + assert.NilError(t, err) + assert.Equal(t, 3600, n) + n, err = parseDuration("1H") + assert.NilError(t, err) + assert.Equal(t, 3600, n) + }) + + t.Run("describes duration", func(t *testing.T) { + assert.Equal(t, "0s", durationDescription(0)) + assert.Equal(t, "-1s", durationDescription(-1)) + assert.Equal(t, "13s", durationDescription(13)) + assert.Equal(t, "1m", durationDescription(60)) + assert.Equal(t, "1m1s", durationDescription(61)) + assert.Equal(t, "1h", durationDescription(3600)) + assert.Equal(t, "1h1s", durationDescription(3601)) + assert.Equal(t, "1h1m", durationDescription(3660)) + assert.Equal(t, "1h1m2s", durationDescription(3662)) + }) + + t.Run("failed to enable profiling", func(t *testing.T) { + cm := &corev1.ConfigMap{} + cmd, client := newProfileCommandWith(cm) + client.CoreV1().(*k8sfakecorev1.FakeCoreV1).PrependReactor("get", "configmaps", + func(action k8stesting.Action) (handled bool, ret k8srt.Object, err error) { + return true, &corev1.ConfigMap{}, errors.New("error getting configmap") + }) + _, err := testutil.ExecuteCommand(cmd, "--enable") + client.CoreV1().(*k8sfakecorev1.FakeCoreV1).PrependReactor("get", "configmaps", + func(action k8stesting.Action) (handled bool, ret k8srt.Object, err error) { + return true, &corev1.ConfigMap{}, errors.New("error getting configmap") + }) + assert.ErrorContains(t, err, "error getting configmap", err) + }) + + t.Run("failed to disable profiling", func(t *testing.T) { + cm := &corev1.ConfigMap{} + cmd, client := newProfileCommandWith(cm) + client.CoreV1().(*k8sfakecorev1.FakeCoreV1).PrependReactor("get", "configmaps", + func(action k8stesting.Action) (handled bool, ret k8srt.Object, err error) { + return true, &corev1.ConfigMap{}, errors.New("error getting configmap") + }) + _, err := testutil.ExecuteCommand(cmd, "--disable") + assert.ErrorContains(t, err, "error getting configmap", err) + }) + + t.Run("successfully enabled profiling", func(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, + Data: map[string]string{"profiling.enable": "false"}, + } + cmd, client := newProfileCommandWith(cm) + out, err := testutil.ExecuteCommand(cmd, "--enable") + assert.NilError(t, err) + assert.Check(t, strings.Contains(out, "Knative profiling is enabled")) + + newCm, err := client.CoreV1().ConfigMaps(knNamespace).Get(obsConfigMap, metav1.GetOptions{}) + assert.NilError(t, err) + assert.Equal(t, "true", newCm.Data["profiling.enable"]) + }) + + t.Run("successfully disabled profiling", func(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, + Data: map[string]string{"profiling.enable": "true"}, + } + cmd, client := newProfileCommandWith(cm) + out, err := testutil.ExecuteCommand(cmd, "--disable") + assert.NilError(t, err) + assert.Check(t, strings.Contains(out, "Knative profiling is disabled")) + + newCm, err := client.CoreV1().ConfigMaps(knNamespace).Get(obsConfigMap, metav1.GetOptions{}) + assert.NilError(t, err) + assert.Equal(t, "false", newCm.Data["profiling.enable"]) + }) + + t.Run("save path folder does not exist", func(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, + Data: map[string]string{"profiling.enable": "true"}, + } + cmd, _ := newProfileCommandWith(cm) + savePath := "/tmp/xsidsk2hsdks" + + _, err := testutil.ExecuteCommand(cmd, "--target", "activator", "--heap", "--save-to", savePath) + assert.ErrorContains(t, err, fmt.Sprintf("the specified save path '%s' doesn't exist", savePath), err) + }) + + t.Run("save path is not a folder", func(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, + Data: map[string]string{"profiling.enable": "true"}, + } + cmd, _ := newProfileCommandWith(cm) + _, filename, _, _ := runtime.Caller(0) + savePath := filename + + _, err := testutil.ExecuteCommand(cmd, "--target", "activator", "--heap", "--save-to", savePath) + assert.ErrorContains(t, err, fmt.Sprintf("the specified save path '%s' is not a folder", savePath), err) + }) + + t.Run("profiling is not enabled when download data", func(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, + Data: map[string]string{"profiling.enable": "false"}, + } + cmd, _ := newProfileCommandWith(cm) + _, err := testutil.ExecuteCommand(cmd, "--target", "activator", "--heap") + assert.ErrorContains(t, err, "profiling is not enabled, please use '--enable' to enalbe it first", err) + }) + + t.Run("failed to get target", func(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, + Data: map[string]string{"profiling.enable": "true"}, + } + cmd, client := newProfileCommandWith(cm) + client.CoreV1().(*k8sfakecorev1.FakeCoreV1).PrependReactor("list", "pods", + func(action k8stesting.Action) (handled bool, ret k8srt.Object, err error) { + return true, &corev1.PodList{}, errors.New("error listing pods") + }) + _, err := testutil.ExecuteCommand(cmd, "--target", "activator", "--heap") + assert.ErrorContains(t, err, "error listing pods", err) + }) + + t.Run("target is not found", func(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, + Data: map[string]string{"profiling.enable": "true"}, + } + cmd, client := newProfileCommandWith(cm) + client.CoreV1().(*k8sfakecorev1.FakeCoreV1).PrependReactor("list", "pods", + func(action k8stesting.Action) (handled bool, ret k8srt.Object, err error) { + return true, &corev1.PodList{}, nil + }) + _, err := testutil.ExecuteCommand(cmd, "--target", "activator", "--heap") + assert.ErrorContains(t, err, "fail to get profiling target 'activator'", err) + }) + + t.Run("failed to get downloader", func(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, + Data: map[string]string{"profiling.enable": "true"}, + } + cmd, client := newProfileCommandWith(cm) + pods := corev1.PodList{Items: []corev1.Pod{ + { + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + ObjectMeta: metav1.ObjectMeta{ + Name: "activator-1", + Namespace: knNamespace, + Labels: map[string]string{ + "app": "activator"}, + }, + }, + }} + client.CoreV1().(*k8sfakecorev1.FakeCoreV1).PrependReactor("list", "pods", + func(action k8stesting.Action) (handled bool, ret k8srt.Object, err error) { + return true, &pods, nil + }) + newDownloaderFunc = fakeDownloaderBuilder(errors.New("error creating downloader"), nil) + defer func() { newDownloaderFunc = NewDownloader }() + + _, err := testutil.ExecuteCommand(cmd, "--target", "activator", "--heap") + assert.ErrorContains(t, err, "error creating downloader", err) + }) + + t.Run("failed to download data", func(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, + Data: map[string]string{"profiling.enable": "true"}, + } + cmd, client := newProfileCommandWith(cm) + cwd, _ := os.Getwd() + podName := "activator-0xxxx" + pods := corev1.PodList{Items: []corev1.Pod{ + { + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: knNamespace, + Labels: map[string]string{ + "app": "activator"}, + }, + }, + }} + client.CoreV1().(*k8sfakecorev1.FakeCoreV1).PrependReactor("list", "pods", + func(action k8stesting.Action) (handled bool, ret k8srt.Object, err error) { + return true, &pods, nil + }) + newDownloaderFunc = fakeDownloaderBuilder(nil, errors.New("error downloading data")) + defer func() { + newDownloaderFunc = NewDownloader + removeProfileDataFiles(filepath.Join(cwd, podName+"_*")) + }() + + _, err := testutil.ExecuteCommand(cmd, "--target", podName, "--heap") + assert.ErrorContains(t, err, "error downloading data", err) + }) + + t.Run("successfully downloaded profile data for a specific pod target", func(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, + Data: map[string]string{"profiling.enable": "true"}, + } + cmd, client := newProfileCommandWith(cm) + cwd, _ := os.Getwd() + podName := "activator-1xxx" + pods := corev1.PodList{Items: []corev1.Pod{ + { + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: knNamespace, + Labels: map[string]string{ + "app": "activator"}, + }, + }, + }} + client.CoreV1().(*k8sfakecorev1.FakeCoreV1).PrependReactor("list", "pods", + func(action k8stesting.Action) (handled bool, ret k8srt.Object, err error) { + return true, &pods, nil + }) + newDownloaderFunc = fakeDownloaderBuilder(nil, nil) + defer func() { + newDownloaderFunc = NewDownloader + removeProfileDataFiles(filepath.Join(cwd, podName+"_*")) + }() + + out, err := testutil.ExecuteCommand(cmd, "--target", podName, "--cpu", "10") + assert.NilError(t, err) + expectedMsg := fmt.Sprintf("Saving 10 second(s) cpu profile data to %s_cpu", filepath.Join(cwd, podName)) + assert.Check(t, strings.Contains(out, expectedMsg), "expected saving cpu profile data output for"+podName) + }) + + t.Run("successfully downloaded profile data for a knative component", func(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, + Data: map[string]string{"profiling.enable": "true"}, + } + cmd, client := newProfileCommandWith(cm) + cwd, _ := os.Getwd() + podNames := []string{"activator-2xxx0", "activator-2xxx1"} + pods := corev1.PodList{} + for _, name := range podNames { + pods.Items = append(pods.Items, corev1.Pod{ + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: knNamespace, + Labels: map[string]string{ + "app": "activator"}, + }, + }) + } + client.CoreV1().(*k8sfakecorev1.FakeCoreV1).PrependReactor("list", "pods", + func(action k8stesting.Action) (handled bool, ret k8srt.Object, err error) { + return true, &pods, nil + }) + newDownloaderFunc = fakeDownloaderBuilder(nil, nil) + defer func() { + newDownloaderFunc = NewDownloader + for _, name := range podNames { + removeProfileDataFiles(filepath.Join(cwd, name+"_*")) + } + }() + + out, err := testutil.ExecuteCommand(cmd, "--target", "activator", "--heap") + assert.NilError(t, err) + for _, name := range podNames { + expectedMsg := fmt.Sprintf("Saving heap profile data to %s_heap", filepath.Join(cwd, name)) + assert.Check(t, strings.Contains(out, expectedMsg), "expected saving heap profile data output for "+name) + } + }) + + t.Run("successfully downloaded multiple profile types data", func(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, + Data: map[string]string{"profiling.enable": "true"}, + } + cmd, client := newProfileCommandWith(cm) + cwd, _ := os.Getwd() + podName := "activator-3xxxx" + pods := corev1.PodList{Items: []corev1.Pod{ + { + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: knNamespace, + Labels: map[string]string{ + "app": "activator"}, + }, + }, + }} + client.CoreV1().(*k8sfakecorev1.FakeCoreV1).PrependReactor("list", "pods", + func(action k8stesting.Action) (handled bool, ret k8srt.Object, err error) { + return true, &pods, nil + }) + newDownloaderFunc = fakeDownloaderBuilder(nil, nil) + defer func() { + newDownloaderFunc = NewDownloader + removeProfileDataFiles(filepath.Join(cwd, podName+"_*")) + }() + + out, err := testutil.ExecuteCommand(cmd, "--target", podName, "--cpu", "8s", "--block", "--mutex") + assert.NilError(t, err) + saveFile := filepath.Join(cwd, podName) + assert.Check(t, strings.Contains(out, "Saving 8 second(s) cpu profile data to "+saveFile+"_cpu"), "expected saving cpu profile data output for "+podName) + assert.Check(t, strings.Contains(out, "Saving block profile data to "+saveFile+"_block"), "expected saving cpu profile data output for "+podName) + assert.Check(t, strings.Contains(out, "Saving mutex profile data to "+saveFile+"_mutex"), "expected saving cpu profile data output for "+podName) + }) + + t.Run("successfully downloaded all profile types data", func(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, + Data: map[string]string{"profiling.enable": "true"}, + } + cmd, client := newProfileCommandWith(cm) + cwd, _ := os.Getwd() + podName := "activator-4xxxx" + pods := corev1.PodList{Items: []corev1.Pod{ + { + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: knNamespace, + Labels: map[string]string{ + "app": "activator"}, + }, + }, + }} + client.CoreV1().(*k8sfakecorev1.FakeCoreV1).PrependReactor("list", "pods", + func(action k8stesting.Action) (handled bool, ret k8srt.Object, err error) { + return true, &pods, nil + }) + newDownloaderFunc = fakeDownloaderBuilder(nil, nil) + defer func() { + newDownloaderFunc = NewDownloader + removeProfileDataFiles(filepath.Join(cwd, podName+"_*")) + }() + + out, err := testutil.ExecuteCommand(cmd, "--target", podName, "--all") + assert.NilError(t, err) + saveFile := filepath.Join(cwd, podName) + expectedMsgs := map[string]string{ + "cpu": fmt.Sprintf("Saving %d second(s) cpu profile data to %s_cpu", defaultDuration, saveFile), + "heap": fmt.Sprintf("Saving heap profile data to %s_heap", saveFile), + "block": fmt.Sprintf("Saving block profile data to %s_block", saveFile), + "trace": fmt.Sprintf("Saving %d second(s) trace profile data to %s_trace", defaultDuration, saveFile), + "mem-allocs": fmt.Sprintf("Saving mem-allocs profile data to %s_mem-allocs", saveFile), + "mutex": fmt.Sprintf("Saving mutex profile data to %s_mutex", saveFile), + "goroutine": fmt.Sprintf("Saving goroutine profile data to %s_goroutine", saveFile), + "thread-create": fmt.Sprintf("Saving thread-create profile data to %s_thread-create", saveFile), + } + for k, v := range expectedMsgs { + assert.Check(t, strings.Contains(out, v), fmt.Sprintf("expected saving %s profile data output for %s", k, podName)) + } + }) +} From ac3f2c201e0d08a406d4e38d24c40980d9c0b583 Mon Sep 17 00:00:00 2001 From: chaozbj Date: Sat, 25 Jul 2020 17:14:52 +0800 Subject: [PATCH 3/5] reworded test function names Signed-off-by: chaozbj --- .../pkg/command/profiling/profiling_test.go | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/plugins/admin/pkg/command/profiling/profiling_test.go b/plugins/admin/pkg/command/profiling/profiling_test.go index ae28f99e..530a8384 100644 --- a/plugins/admin/pkg/command/profiling/profiling_test.go +++ b/plugins/admin/pkg/command/profiling/profiling_test.go @@ -38,13 +38,13 @@ import ( "knative.dev/client-contrib/plugins/admin/pkg/testutil" ) -func newProfileCommand() *cobra.Command { +func newProfilingCommand() *cobra.Command { client := k8sfake.NewSimpleClientset(&corev1.ConfigMap{}) p := pkg.AdminParams{ClientSet: client} return NewProfilingCommand(&p) } -func newProfileCommandWith(cm *corev1.ConfigMap) (*cobra.Command, *k8sfake.Clientset) { +func newProfilingCommandWith(cm *corev1.ConfigMap) (*cobra.Command, *k8sfake.Clientset) { client := k8sfake.NewSimpleClientset(cm) p := pkg.AdminParams{ClientSet: client} return NewProfilingCommand(&p), client @@ -76,14 +76,14 @@ func removeProfileDataFiles(nameFilter string) { // TestNewProfilingCommand tests the profiling command func TestNewProfilingCommand(t *testing.T) { t.Run("runs profiling without args", func(t *testing.T) { - out, err := testutil.ExecuteCommand(newProfileCommand(), "", "") + out, err := testutil.ExecuteCommand(newProfilingCommand(), "", "") assert.NilError(t, err) assert.Check(t, strings.Contains(out, " profiling [flags]"), "expected profiling help output") }) t.Run("runs profiling with conflict args", func(t *testing.T) { // --enable and --disable can't be used together - _, err := testutil.ExecuteCommand(newProfileCommand(), "--enable", "--disable") + _, err := testutil.ExecuteCommand(newProfilingCommand(), "--enable", "--disable") assert.ErrorContains(t, err, "flags '--enable' and '--disable' can not be used together", err) // --enable or --disable --target can't be used with other flags @@ -112,7 +112,7 @@ func TestNewProfilingCommand(t *testing.T) { {"--disable", "--thread-create"}, } for _, args := range argsList { - _, err := testutil.ExecuteCommand(newProfileCommand(), args...) + _, err := testutil.ExecuteCommand(newProfilingCommand(), args...) assert.ErrorContains(t, err, "flag '--enable' or '--disable' can not be used with other flags", err) } @@ -139,7 +139,7 @@ func TestNewProfilingCommand(t *testing.T) { {"--thread-create", "--save-to", "/tmp"}, } for _, args := range argsList { - _, err := testutil.ExecuteCommand(newProfileCommand(), args...) + _, err := testutil.ExecuteCommand(newProfilingCommand(), args...) assert.ErrorContains(t, err, "requires '--target' flag", err) } @@ -149,7 +149,7 @@ func TestNewProfilingCommand(t *testing.T) { {"--target", "activator", "--save-to", "/tmp"}, } for _, args := range argsList { - _, err := testutil.ExecuteCommand(newProfileCommand(), args...) + _, err := testutil.ExecuteCommand(newProfilingCommand(), args...) assert.ErrorContains(t, err, "requires '--all' or a specific profile type flag", err) } }) @@ -221,7 +221,7 @@ func TestNewProfilingCommand(t *testing.T) { t.Run("failed to enable profiling", func(t *testing.T) { cm := &corev1.ConfigMap{} - cmd, client := newProfileCommandWith(cm) + cmd, client := newProfilingCommandWith(cm) client.CoreV1().(*k8sfakecorev1.FakeCoreV1).PrependReactor("get", "configmaps", func(action k8stesting.Action) (handled bool, ret k8srt.Object, err error) { return true, &corev1.ConfigMap{}, errors.New("error getting configmap") @@ -236,7 +236,7 @@ func TestNewProfilingCommand(t *testing.T) { t.Run("failed to disable profiling", func(t *testing.T) { cm := &corev1.ConfigMap{} - cmd, client := newProfileCommandWith(cm) + cmd, client := newProfilingCommandWith(cm) client.CoreV1().(*k8sfakecorev1.FakeCoreV1).PrependReactor("get", "configmaps", func(action k8stesting.Action) (handled bool, ret k8srt.Object, err error) { return true, &corev1.ConfigMap{}, errors.New("error getting configmap") @@ -250,7 +250,7 @@ func TestNewProfilingCommand(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, Data: map[string]string{"profiling.enable": "false"}, } - cmd, client := newProfileCommandWith(cm) + cmd, client := newProfilingCommandWith(cm) out, err := testutil.ExecuteCommand(cmd, "--enable") assert.NilError(t, err) assert.Check(t, strings.Contains(out, "Knative profiling is enabled")) @@ -265,7 +265,7 @@ func TestNewProfilingCommand(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, Data: map[string]string{"profiling.enable": "true"}, } - cmd, client := newProfileCommandWith(cm) + cmd, client := newProfilingCommandWith(cm) out, err := testutil.ExecuteCommand(cmd, "--disable") assert.NilError(t, err) assert.Check(t, strings.Contains(out, "Knative profiling is disabled")) @@ -280,7 +280,7 @@ func TestNewProfilingCommand(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, Data: map[string]string{"profiling.enable": "true"}, } - cmd, _ := newProfileCommandWith(cm) + cmd, _ := newProfilingCommandWith(cm) savePath := "/tmp/xsidsk2hsdks" _, err := testutil.ExecuteCommand(cmd, "--target", "activator", "--heap", "--save-to", savePath) @@ -292,7 +292,7 @@ func TestNewProfilingCommand(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, Data: map[string]string{"profiling.enable": "true"}, } - cmd, _ := newProfileCommandWith(cm) + cmd, _ := newProfilingCommandWith(cm) _, filename, _, _ := runtime.Caller(0) savePath := filename @@ -305,7 +305,7 @@ func TestNewProfilingCommand(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, Data: map[string]string{"profiling.enable": "false"}, } - cmd, _ := newProfileCommandWith(cm) + cmd, _ := newProfilingCommandWith(cm) _, err := testutil.ExecuteCommand(cmd, "--target", "activator", "--heap") assert.ErrorContains(t, err, "profiling is not enabled, please use '--enable' to enalbe it first", err) }) @@ -315,7 +315,7 @@ func TestNewProfilingCommand(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, Data: map[string]string{"profiling.enable": "true"}, } - cmd, client := newProfileCommandWith(cm) + cmd, client := newProfilingCommandWith(cm) client.CoreV1().(*k8sfakecorev1.FakeCoreV1).PrependReactor("list", "pods", func(action k8stesting.Action) (handled bool, ret k8srt.Object, err error) { return true, &corev1.PodList{}, errors.New("error listing pods") @@ -329,7 +329,7 @@ func TestNewProfilingCommand(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, Data: map[string]string{"profiling.enable": "true"}, } - cmd, client := newProfileCommandWith(cm) + cmd, client := newProfilingCommandWith(cm) client.CoreV1().(*k8sfakecorev1.FakeCoreV1).PrependReactor("list", "pods", func(action k8stesting.Action) (handled bool, ret k8srt.Object, err error) { return true, &corev1.PodList{}, nil @@ -343,7 +343,7 @@ func TestNewProfilingCommand(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, Data: map[string]string{"profiling.enable": "true"}, } - cmd, client := newProfileCommandWith(cm) + cmd, client := newProfilingCommandWith(cm) pods := corev1.PodList{Items: []corev1.Pod{ { Status: corev1.PodStatus{Phase: corev1.PodRunning}, @@ -371,7 +371,7 @@ func TestNewProfilingCommand(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, Data: map[string]string{"profiling.enable": "true"}, } - cmd, client := newProfileCommandWith(cm) + cmd, client := newProfilingCommandWith(cm) cwd, _ := os.Getwd() podName := "activator-0xxxx" pods := corev1.PodList{Items: []corev1.Pod{ @@ -404,7 +404,7 @@ func TestNewProfilingCommand(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, Data: map[string]string{"profiling.enable": "true"}, } - cmd, client := newProfileCommandWith(cm) + cmd, client := newProfilingCommandWith(cm) cwd, _ := os.Getwd() podName := "activator-1xxx" pods := corev1.PodList{Items: []corev1.Pod{ @@ -439,7 +439,7 @@ func TestNewProfilingCommand(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, Data: map[string]string{"profiling.enable": "true"}, } - cmd, client := newProfileCommandWith(cm) + cmd, client := newProfilingCommandWith(cm) cwd, _ := os.Getwd() podNames := []string{"activator-2xxx0", "activator-2xxx1"} pods := corev1.PodList{} @@ -479,7 +479,7 @@ func TestNewProfilingCommand(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, Data: map[string]string{"profiling.enable": "true"}, } - cmd, client := newProfileCommandWith(cm) + cmd, client := newProfilingCommandWith(cm) cwd, _ := os.Getwd() podName := "activator-3xxxx" pods := corev1.PodList{Items: []corev1.Pod{ @@ -516,7 +516,7 @@ func TestNewProfilingCommand(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: obsConfigMap, Namespace: knNamespace}, Data: map[string]string{"profiling.enable": "true"}, } - cmd, client := newProfileCommandWith(cm) + cmd, client := newProfilingCommandWith(cm) cwd, _ := os.Getwd() podName := "activator-4xxxx" pods := corev1.PodList{Items: []corev1.Pod{ From e97f9075ffa126e81b0f288b483fdcc1d06b914c Mon Sep 17 00:00:00 2001 From: chaozbj Date: Sat, 25 Jul 2020 17:26:01 +0800 Subject: [PATCH 4/5] improved expected messages for UT Signed-off-by: chaozbj --- plugins/admin/pkg/command/profiling/profiling_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/admin/pkg/command/profiling/profiling_test.go b/plugins/admin/pkg/command/profiling/profiling_test.go index 530a8384..4e0b47b9 100644 --- a/plugins/admin/pkg/command/profiling/profiling_test.go +++ b/plugins/admin/pkg/command/profiling/profiling_test.go @@ -507,8 +507,8 @@ func TestNewProfilingCommand(t *testing.T) { assert.NilError(t, err) saveFile := filepath.Join(cwd, podName) assert.Check(t, strings.Contains(out, "Saving 8 second(s) cpu profile data to "+saveFile+"_cpu"), "expected saving cpu profile data output for "+podName) - assert.Check(t, strings.Contains(out, "Saving block profile data to "+saveFile+"_block"), "expected saving cpu profile data output for "+podName) - assert.Check(t, strings.Contains(out, "Saving mutex profile data to "+saveFile+"_mutex"), "expected saving cpu profile data output for "+podName) + assert.Check(t, strings.Contains(out, "Saving block profile data to "+saveFile+"_block"), "expected saving block profile data output for "+podName) + assert.Check(t, strings.Contains(out, "Saving mutex profile data to "+saveFile+"_mutex"), "expected saving mutex profile data output for "+podName) }) t.Run("successfully downloaded all profile types data", func(t *testing.T) { From 344ec6e5748be2814b3d067913a34f7244b03870 Mon Sep 17 00:00:00 2001 From: chaozbj Date: Sat, 25 Jul 2020 19:58:20 +0800 Subject: [PATCH 5/5] improved command help Signed-off-by: chaozbj --- plugins/admin/README.adoc | 4 ++-- plugins/admin/pkg/command/profiling/profiling.go | 4 ++-- plugins/admin/pkg/command/profiling/profiling_test.go | 1 - 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/plugins/admin/README.adoc b/plugins/admin/README.adoc index 07c3d5ae..debbfdaf 100644 --- a/plugins/admin/README.adoc +++ b/plugins/admin/README.adoc @@ -148,7 +148,7 @@ Examples: Flags: --all Download all profile data --block Download go routine blocking data - --cpu string Download cpu profile data, you can specify a profile data duration with 's' for second(s), 'm' for minute(s) and 'h' for hour(s), e.g: '1m' for one minute (default "5") + --cpu string Download cpu profile data, you can specify a profile data duration with 's' for second(s), 'm' for minute(s) and 'h' for hour(s), e.g: '1m' for one minute (default "5s") --disable Disable Knative profiling --enable Enable Knative profiling --goroutine Download stack traces of all current goroutines data @@ -159,7 +159,7 @@ Flags: -s, --save-to string The path to save the downloaded profile data, if not speicifed, the data will be saved in current working folder -t, --target string The profiling target. It can be a Knative component name or a specific pod name, e.g: 'activator' or 'activator-586d468c99-w59cm' --thread-create Download stack traces that led to the creation of new OS threads data - --trace string Download execution trace data, you can specify a trace data duration with 's' for second(s), 'm' for minute(s) and 'h' for hour(s), e.g: '1m' for one minute (default "5") + --trace string Download execution trace data, you can specify a trace data duration with 's' for second(s), 'm' for minute(s) and 'h' for hour(s), e.g: '1m' for one minute (default "5s") Global Flags: --config string config file (default is $HOME/.config/kn/plugins/admin.yaml) diff --git a/plugins/admin/pkg/command/profiling/profiling.go b/plugins/admin/pkg/command/profiling/profiling.go index 6e084d13..8dc6d58b 100644 --- a/plugins/admin/pkg/command/profiling/profiling.go +++ b/plugins/admin/pkg/command/profiling/profiling.go @@ -169,10 +169,10 @@ func NewProfilingCommand(p *pkg.AdminParams) *cobra.Command { flags.StringVarP(&pflags.target, "target", "t", "", targetFlagUsgae) flags.StringVarP(&pflags.saveTo, "save-to", "s", "", saveToFlagUsage) flags.BoolVar(&pflags.allProfiles, "all", false, allFlagUsage) - flags.StringVarP(&pflags.cpuProfile, cpuFlagName, "", "5", cpuFlagUsage) + flags.StringVarP(&pflags.cpuProfile, cpuFlagName, "", "5s", cpuFlagUsage) flags.BoolVar(&pflags.heapProfile, heapFlagName, false, heapFlagUsage) flags.BoolVar(&pflags.blockProfile, blockFlagName, false, blockFlagUsage) - flags.StringVarP(&pflags.traceProfile, traceFlagName, "", "5", traceFlagUsage) + flags.StringVarP(&pflags.traceProfile, traceFlagName, "", "5s", traceFlagUsage) flags.BoolVar(&pflags.memAllocsProfile, memAllocsFlagName, false, memAllocsFlagUsage) flags.BoolVar(&pflags.mutexProfile, mutexFlagName, false, mutexFlagUsage) flags.BoolVar(&pflags.goroutineProfile, goroutineFlagName, false, goroutineFlagUsage) diff --git a/plugins/admin/pkg/command/profiling/profiling_test.go b/plugins/admin/pkg/command/profiling/profiling_test.go index 4e0b47b9..f23ee0a2 100644 --- a/plugins/admin/pkg/command/profiling/profiling_test.go +++ b/plugins/admin/pkg/command/profiling/profiling_test.go @@ -34,7 +34,6 @@ import ( "k8s.io/client-go/rest" k8stesting "k8s.io/client-go/testing" "knative.dev/client-contrib/plugins/admin/pkg" - "knative.dev/client-contrib/plugins/admin/pkg/testutil" )