Merge pull request #4141 from crazy-max/23.0_backport_fix-perf-reg

[23.0 backport] improve and load plugin command stubs when required
This commit is contained in:
Sebastiaan van Stijn 2023-04-05 02:33:19 +02:00 committed by GitHub
commit 391b2f0fab
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 325 additions and 96 deletions

View File

@ -74,7 +74,7 @@ func TestValidateCandidate(t *testing.T) {
{name: "experimental + allowing experimental", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: metaExperimental}},
} {
t.Run(tc.name, func(t *testing.T) {
p, err := newPlugin(tc.c, fakeroot)
p, err := newPlugin(tc.c, fakeroot.Commands())
if tc.err != "" {
assert.ErrorContains(t, err, tc.err)
} else if tc.invalid != "" {

View File

@ -3,6 +3,7 @@ package manager
import (
"fmt"
"os"
"sync"
"github.com/docker/cli/cli/command"
"github.com/spf13/cobra"
@ -31,13 +32,17 @@ 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)
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 err
return
}
for _, p := range plugins {
p := p
@ -62,8 +67,8 @@ func AddPluginCommandStubs(dockerCli command.Cli, rootCmd *cobra.Command) error
RunE: func(cmd *cobra.Command, args []string) error {
flags := rootCmd.PersistentFlags()
flags.SetOutput(nil)
err := flags.Parse(args)
if err != nil {
perr := flags.Parse(args)
if perr != nil {
return err
}
if flags.Changed("help") {
@ -78,17 +83,18 @@ func AddPluginCommandStubs(dockerCli command.Cli, rootCmd *cobra.Command) error
cargs = append(cargs, args...)
cargs = append(cargs, toComplete)
os.Args = cargs
runCommand, err := PluginRunCommand(dockerCli, p.Name, cmd)
if err != nil {
runCommand, runErr := PluginRunCommand(dockerCli, p.Name, cmd)
if runErr != nil {
return nil, cobra.ShellCompDirectiveError
}
err = runCommand.Run()
if err == nil {
runErr = runCommand.Run()
if runErr == nil {
os.Exit(0) // plugin already rendered complete data
}
return nil, cobra.ShellCompDirectiveError
},
})
}
return nil
})
return err
}

View File

@ -1,15 +1,18 @@
package manager
import (
"context"
"os"
"path/filepath"
"sort"
"strings"
"sync"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/config"
"github.com/fvbommel/sortorder"
"github.com/spf13/cobra"
"golang.org/x/sync/errgroup"
exec "golang.org/x/sys/execabs"
)
@ -120,7 +123,7 @@ func GetPlugin(name string, dockerCli command.Cli, rootcmd *cobra.Command) (*Plu
return nil, errPluginNotFound(name)
}
c := &candidate{paths[0]}
p, err := newPlugin(c, rootcmd)
p, err := newPlugin(c, rootcmd.Commands())
if err != nil {
return nil, err
}
@ -146,19 +149,32 @@ func ListPlugins(dockerCli command.Cli, rootcmd *cobra.Command) ([]Plugin, error
}
var plugins []Plugin
var mu sync.Mutex
eg, _ := errgroup.WithContext(context.TODO())
cmds := rootcmd.Commands()
for _, paths := range candidates {
func(paths []string) {
eg.Go(func() error {
if len(paths) == 0 {
continue
return nil
}
c := &candidate{paths[0]}
p, err := newPlugin(c, rootcmd)
p, err := newPlugin(c, cmds)
if err != nil {
return nil, err
return err
}
if !IsNotFound(p.Err) {
p.ShadowedPaths = paths[1:]
mu.Lock()
defer mu.Unlock()
plugins = append(plugins, p)
}
return nil
})
}(paths)
}
if err := eg.Wait(); err != nil {
return nil, err
}
sort.Slice(plugins, func(i, j int) bool {
@ -199,7 +215,7 @@ func PluginRunCommand(dockerCli command.Cli, name string, rootcmd *cobra.Command
}
c := &candidate{path: path}
plugin, err := newPlugin(c, rootcmd)
plugin, err := newPlugin(c, rootcmd.Commands())
if err != nil {
return nil, err
}

View File

@ -31,7 +31,7 @@ type Plugin struct {
// is set, and is always a `pluginError`, but the `Plugin` is still
// returned with no error. An error is only returned due to a
// non-recoverable error.
func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
func newPlugin(c Candidate, cmds []*cobra.Command) (Plugin, error) {
path := c.Path()
if path == "" {
return Plugin{}, errors.New("plugin candidate path cannot be empty")
@ -62,8 +62,7 @@ func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
return p, nil
}
if rootcmd != nil {
for _, cmd := range rootcmd.Commands() {
for _, cmd := range cmds {
// Ignore conflicts with commands which are
// just plugin stubs (i.e. from a previous
// call to AddPluginCommandStubs).
@ -79,7 +78,6 @@ func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
return p, nil
}
}
}
// We are supposed to check for relevant execute permissions here. Instead we rely on an attempt to execute.
meta, err := c.Metadata()

View File

@ -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",

View File

@ -133,14 +133,21 @@ 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)
}
cmd.PrintErrf("unknown help topic: %v\n", ccmd.Name())
return
}
}
if err := isSupported(ccmd, dockerCli); err != nil {
ccmd.Println(err)
@ -227,10 +234,15 @@ func runDocker(dockerCli *command.DockerCli) error {
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 {
ccmd, _, err := cmd.Find(args)

View File

@ -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,
})
}

View File

@ -37,6 +37,7 @@ require (
github.com/theupdateframework/notary v0.7.1-0.20210315103452-bf96a202a09a
github.com/tonistiigi/go-rosetta v0.0.0-20200727161949-f79598599c5d
github.com/xeipuuv/gojsonschema v1.2.0
golang.org/x/sync v0.1.0
golang.org/x/sys v0.4.0
golang.org/x/term v0.4.0
golang.org/x/text v0.6.0

View File

@ -524,6 +524,8 @@ golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o=
golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=

27
vendor/golang.org/x/sync/LICENSE generated vendored Normal file
View File

@ -0,0 +1,27 @@
Copyright (c) 2009 The Go Authors. All rights reserved.
Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are
met:
* Redistributions of source code must retain the above copyright
notice, this list of conditions and the following disclaimer.
* Redistributions in binary form must reproduce the above
copyright notice, this list of conditions and the following disclaimer
in the documentation and/or other materials provided with the
distribution.
* Neither the name of Google Inc. nor the names of its
contributors may be used to endorse or promote products derived from
this software without specific prior written permission.
THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

22
vendor/golang.org/x/sync/PATENTS generated vendored Normal file
View File

@ -0,0 +1,22 @@
Additional IP Rights Grant (Patents)
"This implementation" means the copyrightable works distributed by
Google as part of the Go project.
Google hereby grants to You a perpetual, worldwide, non-exclusive,
no-charge, royalty-free, irrevocable (except as stated in this section)
patent license to make, have made, use, offer to sell, sell, import,
transfer and otherwise run, modify and propagate the contents of this
implementation of Go, where such license applies only to those patent
claims, both currently owned or controlled by Google and acquired in
the future, licensable by Google that are necessarily infringed by this
implementation of Go. This grant does not include claims that would be
infringed only as a consequence of further modification of this
implementation. If you or your agent or exclusive licensee institute or
order or agree to the institution of patent litigation against any
entity (including a cross-claim or counterclaim in a lawsuit) alleging
that this implementation of Go or any code incorporated within this
implementation of Go constitutes direct or contributory patent
infringement, or inducement of patent infringement, then any patent
rights granted to you under this License for this implementation of Go
shall terminate as of the date such litigation is filed.

132
vendor/golang.org/x/sync/errgroup/errgroup.go generated vendored Normal file
View File

@ -0,0 +1,132 @@
// Copyright 2016 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
// Package errgroup provides synchronization, error propagation, and Context
// cancelation for groups of goroutines working on subtasks of a common task.
package errgroup
import (
"context"
"fmt"
"sync"
)
type token struct{}
// A Group is a collection of goroutines working on subtasks that are part of
// the same overall task.
//
// A zero Group is valid, has no limit on the number of active goroutines,
// and does not cancel on error.
type Group struct {
cancel func()
wg sync.WaitGroup
sem chan token
errOnce sync.Once
err error
}
func (g *Group) done() {
if g.sem != nil {
<-g.sem
}
g.wg.Done()
}
// WithContext returns a new Group and an associated Context derived from ctx.
//
// The derived Context is canceled the first time a function passed to Go
// returns a non-nil error or the first time Wait returns, whichever occurs
// first.
func WithContext(ctx context.Context) (*Group, context.Context) {
ctx, cancel := context.WithCancel(ctx)
return &Group{cancel: cancel}, ctx
}
// Wait blocks until all function calls from the Go method have returned, then
// returns the first non-nil error (if any) from them.
func (g *Group) Wait() error {
g.wg.Wait()
if g.cancel != nil {
g.cancel()
}
return g.err
}
// Go calls the given function in a new goroutine.
// It blocks until the new goroutine can be added without the number of
// active goroutines in the group exceeding the configured limit.
//
// The first call to return a non-nil error cancels the group's context, if the
// group was created by calling WithContext. The error will be returned by Wait.
func (g *Group) Go(f func() error) {
if g.sem != nil {
g.sem <- token{}
}
g.wg.Add(1)
go func() {
defer g.done()
if err := f(); err != nil {
g.errOnce.Do(func() {
g.err = err
if g.cancel != nil {
g.cancel()
}
})
}
}()
}
// TryGo calls the given function in a new goroutine only if the number of
// active goroutines in the group is currently below the configured limit.
//
// The return value reports whether the goroutine was started.
func (g *Group) TryGo(f func() error) bool {
if g.sem != nil {
select {
case g.sem <- token{}:
// Note: this allows barging iff channels in general allow barging.
default:
return false
}
}
g.wg.Add(1)
go func() {
defer g.done()
if err := f(); err != nil {
g.errOnce.Do(func() {
g.err = err
if g.cancel != nil {
g.cancel()
}
})
}
}()
return true
}
// SetLimit limits the number of active goroutines in this group to at most n.
// A negative value indicates no limit.
//
// Any subsequent call to the Go method will block until it can add an active
// goroutine without exceeding the configured limit.
//
// The limit must not be modified while any goroutines in the group are active.
func (g *Group) SetLimit(n int) {
if n < 0 {
g.sem = nil
return
}
if len(g.sem) != 0 {
panic(fmt.Errorf("errgroup: modify limit while %v goroutines in the group are still active", len(g.sem)))
}
g.sem = make(chan token, n)
}

3
vendor/modules.txt vendored
View File

@ -276,6 +276,9 @@ golang.org/x/net/internal/socks
golang.org/x/net/internal/timeseries
golang.org/x/net/proxy
golang.org/x/net/trace
# golang.org/x/sync v0.1.0
## explicit
golang.org/x/sync/errgroup
# golang.org/x/sys v0.4.0
## explicit; go 1.17
golang.org/x/sys/execabs