Skip to content

Commit 6916b42

Browse files
silvin-lubeckisilvin-lubecki
authored
Merge pull request #2774 from thaJeztah/drop_experimental
Always enable experimental features
2 parents 610def4 + 977d3ae commit 6916b42

15 files changed

Lines changed: 71 additions & 126 deletions

File tree

‎cli-plugins/examples/helloworld/main.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,5 @@ func main() {
101101
SchemaVersion: "0.1.0",
102102
Vendor: "Docker Inc.",
103103
Version: "testing",
104-
Experimental: os.Getenv("HELLO_EXPERIMENTAL") != "",
105104
})
106105
}

‎cli-plugins/manager/candidate_test.go‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,9 @@ import (
1212
)
1313

1414
type fakeCandidate struct {
15-
path string
16-
exec bool
17-
meta string
18-
allowExperimental bool
15+
path string
16+
exec bool
17+
meta string
1918
}
2019

2120
func (c *fakeCandidate) Path() string {
@@ -30,7 +29,7 @@ func (c *fakeCandidate) Metadata() ([]byte, error) {
3029
}
3130

3231
func TestValidateCandidate(t *testing.T) {
33-
var (
32+
const (
3433
goodPluginName = NamePrefix + "goodplugin"
3534

3635
builtinName = NamePrefix + "builtin"
@@ -70,14 +69,12 @@ func TestValidateCandidate(t *testing.T) {
7069
{name: "invalid schemaversion", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "xyzzy"}`}, invalid: `plugin SchemaVersion "xyzzy" is not valid`},
7170
{name: "no vendor", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0"}`}, invalid: "plugin metadata does not define a vendor"},
7271
{name: "empty vendor", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": ""}`}, invalid: "plugin metadata does not define a vendor"},
73-
{name: "experimental required", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: metaExperimental}, invalid: "requires experimental CLI"},
7472
// This one should work
7573
{name: "valid", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing"}`}},
76-
{name: "valid + allowing experimental", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing"}`, allowExperimental: true}},
77-
{name: "experimental + allowing experimental", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: metaExperimental, allowExperimental: true}},
74+
{name: "experimental + allowing experimental", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: metaExperimental}},
7875
} {
7976
t.Run(tc.name, func(t *testing.T) {
80-
p, err := newPlugin(tc.c, fakeroot, tc.c.allowExperimental)
77+
p, err := newPlugin(tc.c, fakeroot)
8178
if tc.err != "" {
8279
assert.ErrorContains(t, err, tc.err)
8380
} else if tc.invalid != "" {

‎cli-plugins/manager/manager.go‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package manager
22

33
import (
4-
"fmt"
54
"io/ioutil"
65
"os"
76
"os/exec"
@@ -30,16 +29,6 @@ func (e errPluginNotFound) Error() string {
3029
return "Error: No such CLI plugin: " + string(e)
3130
}
3231

33-
type errPluginRequireExperimental string
34-
35-
// Note: errPluginRequireExperimental implements notFound so that the plugin
36-
// is skipped when listing the plugins.
37-
func (e errPluginRequireExperimental) NotFound() {}
38-
39-
func (e errPluginRequireExperimental) Error() string {
40-
return fmt.Sprintf("plugin candidate %q: requires experimental CLI", string(e))
41-
}
42-
4332
type notFound interface{ NotFound() }
4433

4534
// IsNotFound is true if the given error is due to a plugin not being found.
@@ -133,7 +122,7 @@ func ListPlugins(dockerCli command.Cli, rootcmd *cobra.Command) ([]Plugin, error
133122
continue
134123
}
135124
c := &candidate{paths[0]}
136-
p, err := newPlugin(c, rootcmd, dockerCli.ClientInfo().HasExperimental)
125+
p, err := newPlugin(c, rootcmd)
137126
if err != nil {
138127
return nil, err
139128
}
@@ -181,19 +170,12 @@ func PluginRunCommand(dockerCli command.Cli, name string, rootcmd *cobra.Command
181170
}
182171

183172
c := &candidate{path: path}
184-
plugin, err := newPlugin(c, rootcmd, dockerCli.ClientInfo().HasExperimental)
173+
plugin, err := newPlugin(c, rootcmd)
185174
if err != nil {
186175
return nil, err
187176
}
188177
if plugin.Err != nil {
189178
// TODO: why are we not returning plugin.Err?
190-
191-
err := plugin.Err.(*pluginError).Cause()
192-
// if an experimental plugin was invoked directly while experimental mode is off
193-
// provide a more useful error message than "not found".
194-
if err, ok := err.(errPluginRequireExperimental); ok {
195-
return nil, err
196-
}
197179
return nil, errPluginNotFound(name)
198180
}
199181
cmd := exec.Command(plugin.Path, args...)

‎cli-plugins/manager/metadata.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,6 @@ type Metadata struct {
2323
// URL is a pointer to the plugin's homepage.
2424
URL string `json:",omitempty"`
2525
// Experimental specifies whether the plugin is experimental.
26-
// Experimental plugins are not displayed on non-experimental CLIs.
26+
// Deprecated: experimental features are now always enabled in the CLI
2727
Experimental bool `json:",omitempty"`
2828
}

‎cli-plugins/manager/plugin.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ type Plugin struct {
3535
// non-recoverable error.
3636
//
3737
// nolint: gocyclo
38-
func newPlugin(c Candidate, rootcmd *cobra.Command, allowExperimental bool) (Plugin, error) {
38+
func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
3939
path := c.Path()
4040
if path == "" {
4141
return Plugin{}, errors.New("plugin candidate path cannot be empty")
@@ -96,10 +96,6 @@ func newPlugin(c Candidate, rootcmd *cobra.Command, allowExperimental bool) (Plu
9696
p.Err = wrapAsPluginError(err, "invalid metadata")
9797
return p, nil
9898
}
99-
if p.Experimental && !allowExperimental {
100-
p.Err = &pluginError{errPluginRequireExperimental(p.Name)}
101-
return p, nil
102-
}
10399
if p.Metadata.SchemaVersion != "0.1.0" {
104100
p.Err = NewPluginError("plugin SchemaVersion %q is not valid, must be 0.1.0", p.Metadata.SchemaVersion)
105101
return p, nil

‎cli/cobra.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ func setupCommonRootCommand(rootCmd *cobra.Command) (*cliflags.ClientOptions, *p
3535
cobra.AddTemplateFunc("vendorAndVersion", vendorAndVersion)
3636
cobra.AddTemplateFunc("invalidPluginReason", invalidPluginReason)
3737
cobra.AddTemplateFunc("isPlugin", isPlugin)
38+
cobra.AddTemplateFunc("isExperimental", isExperimental)
3839
cobra.AddTemplateFunc("decoratedName", decoratedName)
3940

4041
rootCmd.SetUsageTemplate(usageTemplate)
@@ -191,6 +192,19 @@ var helpCommand = &cobra.Command{
191192
},
192193
}
193194

195+
func isExperimental(cmd *cobra.Command) bool {
196+
if _, ok := cmd.Annotations["experimentalCLI"]; ok {
197+
return true
198+
}
199+
var experimental bool
200+
cmd.VisitParents(func(cmd *cobra.Command) {
201+
if _, ok := cmd.Annotations["experimentalCLI"]; ok {
202+
experimental = true
203+
}
204+
})
205+
return experimental
206+
}
207+
194208
func isPlugin(cmd *cobra.Command) bool {
195209
return cmd.Annotations[pluginmanager.CommandAnnotationPlugin] == "true"
196210
}
@@ -286,7 +300,16 @@ var usageTemplate = `Usage:
286300
{{- if .HasSubCommands}} {{ .CommandPath}}{{- if .HasAvailableFlags}} [OPTIONS]{{end}} COMMAND{{end}}
287301
288302
{{if ne .Long ""}}{{ .Long | trim }}{{ else }}{{ .Short | trim }}{{end}}
303+
{{- if isExperimental .}}
304+
305+
EXPERIMENTAL:
306+
{{.CommandPath}} is an experimental feature.
307+
Experimental features provide early access to product functionality. These
308+
features may change between releases without warning, or can be removed from a
309+
future release. Learn more about experimental features in our documentation:
310+
https://docs.docker.com/go/experimental/
289311
312+
{{- end}}
290313
{{- if gt .Aliases 0}}
291314
292315
Aliases:

‎cli/command/cli.go‎

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -152,16 +152,6 @@ func (cli *DockerCli) ClientInfo() ClientInfo {
152152
}
153153

154154
func (cli *DockerCli) loadClientInfo() error {
155-
var experimentalValue string
156-
// Environment variable always overrides configuration
157-
if experimentalValue = os.Getenv("DOCKER_CLI_EXPERIMENTAL"); experimentalValue == "" {
158-
experimentalValue = cli.ConfigFile().Experimental
159-
}
160-
hasExperimental, err := isEnabled(experimentalValue)
161-
if err != nil {
162-
return errors.Wrap(err, "Experimental field")
163-
}
164-
165155
var v string
166156
if cli.client != nil {
167157
v = cli.client.ClientVersion()
@@ -170,7 +160,7 @@ func (cli *DockerCli) loadClientInfo() error {
170160
}
171161
cli.clientInfo = &ClientInfo{
172162
DefaultVersion: v,
173-
HasExperimental: hasExperimental,
163+
HasExperimental: true,
174164
}
175165
return nil
176166
}
@@ -358,17 +348,6 @@ func resolveDefaultDockerEndpoint(opts *cliflags.CommonOptions) (docker.Endpoint
358348
}, nil
359349
}
360350

361-
func isEnabled(value string) (bool, error) {
362-
switch value {
363-
case "enabled":
364-
return true, nil
365-
case "", "disabled":
366-
return false, nil
367-
default:
368-
return false, errors.Errorf("%q is not valid, should be either enabled or disabled", value)
369-
}
370-
}
371-
372351
func (cli *DockerCli) initializeFromClient() {
373352
ctx := context.Background()
374353
if strings.HasPrefix(cli.DockerEndpoint().Host, "tcp://") {
@@ -471,6 +450,8 @@ type ServerInfo struct {
471450

472451
// ClientInfo stores details about the supported features of the client
473452
type ClientInfo struct {
453+
// Deprecated: experimental CLI features always enabled. This field is kept
454+
// for backward-compatibility, and is always "true".
474455
HasExperimental bool
475456
DefaultVersion string
476457
}

‎cli/command/cli_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -167,25 +167,24 @@ func TestInitializeFromClient(t *testing.T) {
167167
}
168168
}
169169

170+
// The CLI no longer disables/hides experimental CLI features, however, we need
171+
// to verify that existing configuration files do not break
170172
func TestExperimentalCLI(t *testing.T) {
171173
defaultVersion := "v1.55"
172174

173175
var testcases = []struct {
174-
doc string
175-
configfile string
176-
expectedExperimentalCLI bool
176+
doc string
177+
configfile string
177178
}{
178179
{
179-
doc: "default",
180-
configfile: `{}`,
181-
expectedExperimentalCLI: false,
180+
doc: "default",
181+
configfile: `{}`,
182182
},
183183
{
184184
doc: "experimental",
185185
configfile: `{
186186
"experimental": "enabled"
187187
}`,
188-
expectedExperimentalCLI: true,
189188
},
190189
}
191190

@@ -205,7 +204,8 @@ func TestExperimentalCLI(t *testing.T) {
205204
cliconfig.SetDir(dir.Path())
206205
err := cli.Initialize(flags.NewClientOptions())
207206
assert.NilError(t, err)
208-
assert.Check(t, is.Equal(testcase.expectedExperimentalCLI, cli.ClientInfo().HasExperimental))
207+
// For backward-compatibility, HasExperimental will always be "true"
208+
assert.Check(t, is.Equal(true, cli.ClientInfo().HasExperimental))
209209
})
210210
}
211211
}

‎cli/command/stack/kubernetes/cli.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func WrapCli(dockerCli command.Cli, opts Options) (*KubeCli, error) {
103103
}
104104

105105
func (c *KubeCli) composeClient() (*Factory, error) {
106-
return NewFactory(c.kubeNamespace, c.kubeConfig, c.clientSet, c.ClientInfo().HasExperimental)
106+
return NewFactory(c.kubeNamespace, c.kubeConfig, c.clientSet)
107107
}
108108

109109
func (c *KubeCli) checkHostsMatch() error {

‎cli/command/stack/kubernetes/client.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,10 @@ type Factory struct {
1818
coreClientSet corev1.CoreV1Interface
1919
appsClientSet appsv1beta2.AppsV1beta2Interface
2020
clientSet *kubeclient.Clientset
21-
experimental bool
2221
}
2322

2423
// NewFactory creates a kubernetes client factory
25-
func NewFactory(namespace string, config *restclient.Config, clientSet *kubeclient.Clientset, experimental bool) (*Factory, error) {
24+
func NewFactory(namespace string, config *restclient.Config, clientSet *kubeclient.Clientset) (*Factory, error) {
2625
coreClientSet, err := corev1.NewForConfig(config)
2726
if err != nil {
2827
return nil, err
@@ -39,7 +38,6 @@ func NewFactory(namespace string, config *restclient.Config, clientSet *kubeclie
3938
coreClientSet: coreClientSet,
4039
appsClientSet: appsClientSet,
4140
clientSet: clientSet,
42-
experimental: experimental,
4341
}, nil
4442
}
4543

@@ -85,7 +83,7 @@ func (s *Factory) DaemonSets() typesappsv1beta2.DaemonSetInterface {
8583

8684
// Stacks returns a client for Docker's Stack on Kubernetes
8785
func (s *Factory) Stacks(allNamespaces bool) (StackClient, error) {
88-
version, err := kubernetes.GetStackAPIVersion(s.clientSet.Discovery(), s.experimental)
86+
version, err := kubernetes.GetStackAPIVersion(s.clientSet.Discovery())
8987
if err != nil {
9088
return nil, err
9189
}

0 commit comments

Comments
 (0)