Merge pull request #2774 from thaJeztah/drop_experimental

Always enable experimental features
Upstream-commit: 6916b427a0b07e8581d121967633235ced6db9a1
Component: cli

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

@@ -101,6 +101,5 @@ func main() {
 			SchemaVersion: "0.1.0",
 			Vendor:        "Docker Inc.",
 			Version:       "testing",
-			Experimental:  os.Getenv("HELLO_EXPERIMENTAL") != "",
 		})
 }

components/cli/cli-plugins/manager/candidate_test.go

@@ -12,10 +12,9 @@ import (
 )
 
 type fakeCandidate struct {
-	path              string
-	exec              bool
-	meta              string
-	allowExperimental bool
+	path string
+	exec bool
+	meta string
 }
 
 func (c *fakeCandidate) Path() string {
@@ -30,7 +29,7 @@ func (c *fakeCandidate) Metadata() ([]byte, error) {
 }
 
 func TestValidateCandidate(t *testing.T) {
-	var (
+	const (
 		goodPluginName = NamePrefix + "goodplugin"
 
 		builtinName  = NamePrefix + "builtin"
@@ -70,14 +69,12 @@ func TestValidateCandidate(t *testing.T) {
 		{name: "invalid schemaversion", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "xyzzy"}`}, invalid: `plugin SchemaVersion "xyzzy" is not valid`},
 		{name: "no vendor", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0"}`}, invalid: "plugin metadata does not define a vendor"},
 		{name: "empty vendor", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": ""}`}, invalid: "plugin metadata does not define a vendor"},
-		{name: "experimental required", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: metaExperimental}, invalid: "requires experimental CLI"},
 		// This one should work
 		{name: "valid", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing"}`}},
-		{name: "valid + allowing experimental", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing"}`, allowExperimental: true}},
-		{name: "experimental + allowing experimental", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: metaExperimental, allowExperimental: true}},
+		{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, tc.c.allowExperimental)
+			p, err := newPlugin(tc.c, fakeroot)
 			if tc.err != "" {
 				assert.ErrorContains(t, err, tc.err)
 			} else if tc.invalid != "" {

components/cli/cli-plugins/manager/manager.go

@@ -1,7 +1,6 @@
 package manager
 
 import (
-	"fmt"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -30,16 +29,6 @@ func (e errPluginNotFound) Error() string {
 	return "Error: No such CLI plugin: " + string(e)
 }
 
-type errPluginRequireExperimental string
-
-// Note: errPluginRequireExperimental implements notFound so that the plugin
-// is skipped when listing the plugins.
-func (e errPluginRequireExperimental) NotFound() {}
-
-func (e errPluginRequireExperimental) Error() string {
-	return fmt.Sprintf("plugin candidate %q: requires experimental CLI", string(e))
-}
-
 type notFound interface{ NotFound() }
 
 // 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
 			continue
 		}
 		c := &candidate{paths[0]}
-		p, err := newPlugin(c, rootcmd, dockerCli.ClientInfo().HasExperimental)
+		p, err := newPlugin(c, rootcmd)
 		if err != nil {
 			return nil, err
 		}
@@ -181,19 +170,12 @@ func PluginRunCommand(dockerCli command.Cli, name string, rootcmd *cobra.Command
 		}
 
 		c := &candidate{path: path}
-		plugin, err := newPlugin(c, rootcmd, dockerCli.ClientInfo().HasExperimental)
+		plugin, err := newPlugin(c, rootcmd)
 		if err != nil {
 			return nil, err
 		}
 		if plugin.Err != nil {
 			// TODO: why are we not returning plugin.Err?
-
-			err := plugin.Err.(*pluginError).Cause()
-			// if an experimental plugin was invoked directly while experimental mode is off
-			// provide a more useful error message than "not found".
-			if err, ok := err.(errPluginRequireExperimental); ok {
-				return nil, err
-			}
 			return nil, errPluginNotFound(name)
 		}
 		cmd := exec.Command(plugin.Path, args...)

components/cli/cli-plugins/manager/metadata.go

@@ -23,6 +23,6 @@ type Metadata struct {
 	// URL is a pointer to the plugin's homepage.
 	URL string `json:",omitempty"`
 	// Experimental specifies whether the plugin is experimental.
-	// Experimental plugins are not displayed on non-experimental CLIs.
+	// Deprecated: experimental features are now always enabled in the CLI
 	Experimental bool `json:",omitempty"`
 }

components/cli/cli-plugins/manager/plugin.go

@@ -35,7 +35,7 @@ type Plugin struct {
 // non-recoverable error.
 //
 // nolint: gocyclo
-func newPlugin(c Candidate, rootcmd *cobra.Command, allowExperimental bool) (Plugin, error) {
+func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
 	path := c.Path()
 	if path == "" {
 		return Plugin{}, errors.New("plugin candidate path cannot be empty")
@@ -96,10 +96,6 @@ func newPlugin(c Candidate, rootcmd *cobra.Command, allowExperimental bool) (Plu
 		p.Err = wrapAsPluginError(err, "invalid metadata")
 		return p, nil
 	}
-	if p.Experimental && !allowExperimental {
-		p.Err = &pluginError{errPluginRequireExperimental(p.Name)}
-		return p, nil
-	}
 	if p.Metadata.SchemaVersion != "0.1.0" {
 		p.Err = NewPluginError("plugin SchemaVersion %q is not valid, must be 0.1.0", p.Metadata.SchemaVersion)
 		return p, nil

components/cli/cli/cobra.go

@@ -35,6 +35,7 @@ func setupCommonRootCommand(rootCmd *cobra.Command) (*cliflags.ClientOptions, *p
 	cobra.AddTemplateFunc("vendorAndVersion", vendorAndVersion)
 	cobra.AddTemplateFunc("invalidPluginReason", invalidPluginReason)
 	cobra.AddTemplateFunc("isPlugin", isPlugin)
+	cobra.AddTemplateFunc("isExperimental", isExperimental)
 	cobra.AddTemplateFunc("decoratedName", decoratedName)
 
 	rootCmd.SetUsageTemplate(usageTemplate)
@@ -191,6 +192,19 @@ var helpCommand = &cobra.Command{
 	},
 }
 
+func isExperimental(cmd *cobra.Command) bool {
+	if _, ok := cmd.Annotations["experimentalCLI"]; ok {
+		return true
+	}
+	var experimental bool
+	cmd.VisitParents(func(cmd *cobra.Command) {
+		if _, ok := cmd.Annotations["experimentalCLI"]; ok {
+			experimental = true
+		}
+	})
+	return experimental
+}
+
 func isPlugin(cmd *cobra.Command) bool {
 	return cmd.Annotations[pluginmanager.CommandAnnotationPlugin] == "true"
 }
@@ -286,7 +300,16 @@ var usageTemplate = `Usage:
 {{- if .HasSubCommands}}	{{ .CommandPath}}{{- if .HasAvailableFlags}} [OPTIONS]{{end}} COMMAND{{end}}
 
 {{if ne .Long ""}}{{ .Long | trim }}{{ else }}{{ .Short | trim }}{{end}}
+{{- if isExperimental .}}
+
+EXPERIMENTAL:
+  {{.CommandPath}} is an experimental feature.
+  Experimental features provide early access to product functionality. These
+  features may change between releases without warning, or can be removed from a
+  future release. Learn more about experimental features in our documentation:
+  https://docs.docker.com/go/experimental/
 
+{{- end}}
 {{- if gt .Aliases 0}}
 
 Aliases:

components/cli/cli/command/cli.go

@@ -152,16 +152,6 @@ func (cli *DockerCli) ClientInfo() ClientInfo {
 }
 
 func (cli *DockerCli) loadClientInfo() error {
-	var experimentalValue string
-	// Environment variable always overrides configuration
-	if experimentalValue = os.Getenv("DOCKER_CLI_EXPERIMENTAL"); experimentalValue == "" {
-		experimentalValue = cli.ConfigFile().Experimental
-	}
-	hasExperimental, err := isEnabled(experimentalValue)
-	if err != nil {
-		return errors.Wrap(err, "Experimental field")
-	}
-
 	var v string
 	if cli.client != nil {
 		v = cli.client.ClientVersion()
@@ -170,7 +160,7 @@ func (cli *DockerCli) loadClientInfo() error {
 	}
 	cli.clientInfo = &ClientInfo{
 		DefaultVersion:  v,
-		HasExperimental: hasExperimental,
+		HasExperimental: true,
 	}
 	return nil
 }
@@ -358,17 +348,6 @@ func resolveDefaultDockerEndpoint(opts *cliflags.CommonOptions) (docker.Endpoint
 	}, nil
 }
 
-func isEnabled(value string) (bool, error) {
-	switch value {
-	case "enabled":
-		return true, nil
-	case "", "disabled":
-		return false, nil
-	default:
-		return false, errors.Errorf("%q is not valid, should be either enabled or disabled", value)
-	}
-}
-
 func (cli *DockerCli) initializeFromClient() {
 	ctx := context.Background()
 	if strings.HasPrefix(cli.DockerEndpoint().Host, "tcp://") {
@@ -471,6 +450,8 @@ type ServerInfo struct {
 
 // ClientInfo stores details about the supported features of the client
 type ClientInfo struct {
+	// Deprecated: experimental CLI features always enabled. This field is kept
+	// for backward-compatibility, and is always "true".
 	HasExperimental bool
 	DefaultVersion  string
 }

components/cli/cli/command/cli_test.go

@@ -167,25 +167,24 @@ func TestInitializeFromClient(t *testing.T) {
 	}
 }
 
+// The CLI no longer disables/hides experimental CLI features, however, we need
+// to verify that existing configuration files do not break
 func TestExperimentalCLI(t *testing.T) {
 	defaultVersion := "v1.55"
 
 	var testcases = []struct {
-		doc                     string
-		configfile              string
-		expectedExperimentalCLI bool
+		doc        string
+		configfile string
 	}{
 		{
-			doc:                     "default",
-			configfile:              `{}`,
-			expectedExperimentalCLI: false,
+			doc:        "default",
+			configfile: `{}`,
 		},
 		{
 			doc: "experimental",
 			configfile: `{
 	"experimental": "enabled"
 }`,
-			expectedExperimentalCLI: true,
 		},
 	}
 
@@ -205,7 +204,8 @@ func TestExperimentalCLI(t *testing.T) {
 			cliconfig.SetDir(dir.Path())
 			err := cli.Initialize(flags.NewClientOptions())
 			assert.NilError(t, err)
-			assert.Check(t, is.Equal(testcase.expectedExperimentalCLI, cli.ClientInfo().HasExperimental))
+			// For backward-compatibility, HasExperimental will always be "true"
+			assert.Check(t, is.Equal(true, cli.ClientInfo().HasExperimental))
 		})
 	}
 }

components/cli/cli/command/stack/kubernetes/cli.go

@@ -103,7 +103,7 @@ func WrapCli(dockerCli command.Cli, opts Options) (*KubeCli, error) {
 }
 
 func (c *KubeCli) composeClient() (*Factory, error) {
-	return NewFactory(c.kubeNamespace, c.kubeConfig, c.clientSet, c.ClientInfo().HasExperimental)
+	return NewFactory(c.kubeNamespace, c.kubeConfig, c.clientSet)
 }
 
 func (c *KubeCli) checkHostsMatch() error {

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

@@ -18,11 +18,10 @@ type Factory struct {
 	coreClientSet corev1.CoreV1Interface
 	appsClientSet appsv1beta2.AppsV1beta2Interface
 	clientSet     *kubeclient.Clientset
-	experimental  bool
 }
 
 // NewFactory creates a kubernetes client factory
-func NewFactory(namespace string, config *restclient.Config, clientSet *kubeclient.Clientset, experimental bool) (*Factory, error) {
+func NewFactory(namespace string, config *restclient.Config, clientSet *kubeclient.Clientset) (*Factory, error) {
 	coreClientSet, err := corev1.NewForConfig(config)
 	if err != nil {
 		return nil, err
@@ -39,7 +38,6 @@ func NewFactory(namespace string, config *restclient.Config, clientSet *kubeclie
 		coreClientSet: coreClientSet,
 		appsClientSet: appsClientSet,
 		clientSet:     clientSet,
-		experimental:  experimental,
 	}, nil
 }
 
@@ -85,7 +83,7 @@ func (s *Factory) DaemonSets() typesappsv1beta2.DaemonSetInterface {
 
 // Stacks returns a client for Docker's Stack on Kubernetes
 func (s *Factory) Stacks(allNamespaces bool) (StackClient, error) {
-	version, err := kubernetes.GetStackAPIVersion(s.clientSet.Discovery(), s.experimental)
+	version, err := kubernetes.GetStackAPIVersion(s.clientSet.Discovery())
 	if err != nil {
 		return nil, err
 	}

components/cli/cli/command/system/version.go

@@ -2,9 +2,9 @@ package system
 
 import (
 	"context"
-	"fmt"
 	"runtime"
 	"sort"
+	"strconv"
 	"text/tabwriter"
 	"text/template"
 	"time"
@@ -83,7 +83,7 @@ type clientVersion struct {
 	Arch              string
 	BuildTime         string `json:",omitempty"`
 	Context           string
-	Experimental      bool
+	Experimental      bool `json:",omitempty"` // Deprecated: experimental CLI features always enabled. This field is kept for backward-compatibility, and is always "true"
 }
 
 type kubernetesVersion struct {
@@ -157,7 +157,7 @@ func runVersion(dockerCli command.Cli, opts *versionOptions) error {
 			BuildTime:         reformatDate(version.BuildTime),
 			Os:                runtime.GOOS,
 			Arch:              arch(),
-			Experimental:      dockerCli.ClientInfo().HasExperimental,
+			Experimental:      true,
 			Context:           dockerCli.CurrentContext(),
 		},
 	}
@@ -199,7 +199,7 @@ func runVersion(dockerCli command.Cli, opts *versionOptions) error {
 					"Os":            sv.Os,
 					"Arch":          sv.Arch,
 					"BuildTime":     reformatDate(vd.Server.BuildTime),
-					"Experimental":  fmt.Sprintf("%t", sv.Experimental),
+					"Experimental":  strconv.FormatBool(sv.Experimental),
 				},
 			})
 		}
@@ -274,13 +274,13 @@ func getKubernetesVersion(dockerCli command.Cli, kubeConfig string) *kubernetesV
 		logrus.Debugf("failed to get Kubernetes client: %s", err)
 		return &version
 	}
-	version.StackAPI = getStackVersion(kubeClient, dockerCli.ClientInfo().HasExperimental)
+	version.StackAPI = getStackVersion(kubeClient)
 	version.Kubernetes = getKubernetesServerVersion(kubeClient)
 	return &version
 }
 
-func getStackVersion(client *kubernetesClient.Clientset, experimental bool) string {
-	apiVersion, err := kubernetes.GetStackAPIVersion(client, experimental)
+func getStackVersion(client *kubernetesClient.Clientset) string {
+	apiVersion, err := kubernetes.GetStackAPIVersion(client)
 	if err != nil {
 		logrus.Debugf("failed to get Stack API version: %s", err)
 		return "Unknown"