From d93d78588dc20ea8e218018877dfa58f098efb05 Mon Sep 17 00:00:00 2001 From: Kevin Alvarez Date: Tue, 28 Mar 2023 04:08:07 +0200 Subject: [PATCH] load plugin command stubs when required We are currently loading plugin command stubs for every invocation which still has a significant performance hit. With this change we are doing this operation only if cobra completion arg request is found. - 20.10.23: `docker --version` takes ~15ms - 23.0.1: `docker --version` takes ~93ms With this change `docker --version` takes ~9ms Signed-off-by: CrazyMax (cherry picked from commit c39c711a18cbd9970ac2e040262b830fd6b4e3f9) --- cli-plugins/manager/cobra.go | 116 ++++++++++++++++++----------------- cli/cobra.go | 10 +++ cmd/docker/docker.go | 24 ++++++-- e2e/cli-plugins/run_test.go | 6 +- 4 files changed, 92 insertions(+), 64 deletions(-) diff --git a/cli-plugins/manager/cobra.go b/cli-plugins/manager/cobra.go index 71b6fe716d..be3a058201 100644 --- a/cli-plugins/manager/cobra.go +++ b/cli-plugins/manager/cobra.go @@ -3,6 +3,7 @@ package manager import ( "fmt" "os" + "sync" "github.com/docker/cli/cli/command" "github.com/spf13/cobra" @@ -31,64 +32,69 @@ const ( CommandAnnotationPluginInvalid = "com.docker.cli.plugin-invalid" ) +var pluginCommandStubsOnce sync.Once + // AddPluginCommandStubs adds a stub cobra.Commands for each valid and invalid // plugin. The command stubs will have several annotations added, see // `CommandAnnotationPlugin*`. -func AddPluginCommandStubs(dockerCli command.Cli, rootCmd *cobra.Command) error { - plugins, err := ListPlugins(dockerCli, rootCmd) - if err != nil { - return err - } - for _, p := range plugins { - p := p - vendor := p.Vendor - if vendor == "" { - vendor = "unknown" +func AddPluginCommandStubs(dockerCli command.Cli, rootCmd *cobra.Command) (err error) { + pluginCommandStubsOnce.Do(func() { + var plugins []Plugin + plugins, err = ListPlugins(dockerCli, rootCmd) + if err != nil { + return } - annotations := map[string]string{ - CommandAnnotationPlugin: "true", - CommandAnnotationPluginVendor: vendor, - CommandAnnotationPluginVersion: p.Version, - } - if p.Err != nil { - annotations[CommandAnnotationPluginInvalid] = p.Err.Error() - } - rootCmd.AddCommand(&cobra.Command{ - Use: p.Name, - Short: p.ShortDescription, - Run: func(_ *cobra.Command, _ []string) {}, - Annotations: annotations, - DisableFlagParsing: true, - RunE: func(cmd *cobra.Command, args []string) error { - flags := rootCmd.PersistentFlags() - flags.SetOutput(nil) - err := flags.Parse(args) - if err != nil { - return err - } - if flags.Changed("help") { - cmd.HelpFunc()(rootCmd, args) - return nil - } - return fmt.Errorf("docker: '%s' is not a docker command.\nSee 'docker --help'", cmd.Name()) - }, - ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { - // Delegate completion to plugin - cargs := []string{p.Path, cobra.ShellCompRequestCmd, p.Name} - cargs = append(cargs, args...) - cargs = append(cargs, toComplete) - os.Args = cargs - runCommand, err := PluginRunCommand(dockerCli, p.Name, cmd) - if err != nil { + for _, p := range plugins { + p := p + vendor := p.Vendor + if vendor == "" { + vendor = "unknown" + } + annotations := map[string]string{ + CommandAnnotationPlugin: "true", + CommandAnnotationPluginVendor: vendor, + CommandAnnotationPluginVersion: p.Version, + } + if p.Err != nil { + annotations[CommandAnnotationPluginInvalid] = p.Err.Error() + } + rootCmd.AddCommand(&cobra.Command{ + Use: p.Name, + Short: p.ShortDescription, + Run: func(_ *cobra.Command, _ []string) {}, + Annotations: annotations, + DisableFlagParsing: true, + RunE: func(cmd *cobra.Command, args []string) error { + flags := rootCmd.PersistentFlags() + flags.SetOutput(nil) + perr := flags.Parse(args) + if perr != nil { + return err + } + if flags.Changed("help") { + cmd.HelpFunc()(rootCmd, args) + return nil + } + return fmt.Errorf("docker: '%s' is not a docker command.\nSee 'docker --help'", cmd.Name()) + }, + ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + // Delegate completion to plugin + cargs := []string{p.Path, cobra.ShellCompRequestCmd, p.Name} + cargs = append(cargs, args...) + cargs = append(cargs, toComplete) + os.Args = cargs + runCommand, runErr := PluginRunCommand(dockerCli, p.Name, cmd) + if runErr != nil { + return nil, cobra.ShellCompDirectiveError + } + runErr = runCommand.Run() + if runErr == nil { + os.Exit(0) // plugin already rendered complete data + } return nil, cobra.ShellCompDirectiveError - } - err = runCommand.Run() - if err == nil { - os.Exit(0) // plugin already rendered complete data - } - return nil, cobra.ShellCompDirectiveError - }, - }) - } - return nil + }, + }) + } + }) + return err } diff --git a/cli/cobra.go b/cli/cobra.go index 1b07fc5648..6501197ddd 100644 --- a/cli/cobra.go +++ b/cli/cobra.go @@ -204,6 +204,16 @@ func DisableFlagsInUseLine(cmd *cobra.Command) { }) } +// HasCompletionArg returns true if a cobra completion arg request is found. +func HasCompletionArg(args []string) bool { + for _, arg := range args { + if arg == cobra.ShellCompRequestCmd || arg == cobra.ShellCompNoDescRequestCmd { + return true + } + } + return false +} + var helpCommand = &cobra.Command{ Use: "help [command]", Short: "Help about the command", diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 9becec2c07..d18125a634 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -133,13 +133,20 @@ func tryRunPluginHelp(dockerCli command.Cli, ccmd *cobra.Command, cargs []string func setHelpFunc(dockerCli command.Cli, cmd *cobra.Command) { defaultHelpFunc := cmd.HelpFunc() cmd.SetHelpFunc(func(ccmd *cobra.Command, args []string) { - if pluginmanager.IsPluginCommand(ccmd) { + if err := pluginmanager.AddPluginCommandStubs(dockerCli, ccmd.Root()); err != nil { + ccmd.Println(err) + return + } + + if len(args) >= 1 { err := tryRunPluginHelp(dockerCli, ccmd, args) + if err == nil { + return + } if !pluginmanager.IsNotFound(err) { ccmd.Println(err) + return } - cmd.PrintErrf("unknown help topic: %v\n", ccmd.Name()) - return } if err := isSupported(ccmd, dockerCli); err != nil { @@ -227,9 +234,14 @@ func runDocker(dockerCli *command.DockerCli) error { return err } - err = pluginmanager.AddPluginCommandStubs(dockerCli, cmd) - if err != nil { - return err + if cli.HasCompletionArg(args) { + // We add plugin command stubs early only for completion. We don't + // want to add them for normal command execution as it would cause + // a significant performance hit. + err = pluginmanager.AddPluginCommandStubs(dockerCli, cmd) + if err != nil { + return err + } } if len(args) > 0 { diff --git a/e2e/cli-plugins/run_test.go b/e2e/cli-plugins/run_test.go index 2a2f318f46..d3daa35563 100644 --- a/e2e/cli-plugins/run_test.go +++ b/e2e/cli-plugins/run_test.go @@ -83,7 +83,7 @@ func TestHelpBad(t *testing.T) { res := icmd.RunCmd(run("help", "badmeta")) res.Assert(t, icmd.Expected{ - ExitCode: 0, + ExitCode: 1, Out: icmd.None, }) golden.Assert(t, res.Stderr(), "docker-help-badmeta-err.golden") @@ -110,8 +110,8 @@ func TestBadHelp(t *testing.T) { res.Assert(t, icmd.Expected{ ExitCode: 0, // This should be identical to the --help case above - Out: usage, - Err: shortHFlagDeprecated, + Out: shortHFlagDeprecated + usage, + Err: icmd.None, }) }