From 977d3ae046ec6c64be8788a8712251ed547a2bdb Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 2 Oct 2020 14:19:34 +0200 Subject: [PATCH] Always enable experimental features The CLI disabled experimental features by default, requiring users to set a configuration option to enable them. Disabling experimental features was a request from Enterprise users that did not want experimental features to be accessible. We are changing this policy, and now enable experimental features by default. Experimental features may still change and/or removed, and will be highlighted in the documentation and "usage" output. For example, the `docker manifest inspect --help` output now shows: EXPERIMENTAL: docker manifest inspect is an experimental feature. Experimental features provide early access to product functionality. These features may change between releases without warning or can be removed entirely from a future release. Learn more about experimental features: https://docs.docker.com/go/experimental/ Signed-off-by: Sebastiaan van Stijn --- cli-plugins/examples/helloworld/main.go | 1 - cli-plugins/manager/candidate_test.go | 15 ++++++--------- cli-plugins/manager/manager.go | 22 ++-------------------- cli-plugins/manager/metadata.go | 2 +- cli-plugins/manager/plugin.go | 6 +----- cli/command/cli.go | 25 +++---------------------- cli/command/cli_test.go | 16 ++++++++-------- cli/command/stack/kubernetes/cli.go | 2 +- cli/command/stack/kubernetes/client.go | 6 ++---- cli/command/system/version.go | 14 +++++++------- cmd/docker/docker.go | 19 +++++-------------- docs/reference/commandline/cli.md | 24 ++---------------------- kubernetes/check.go | 8 ++++---- kubernetes/check_test.go | 14 ++++++-------- 14 files changed, 48 insertions(+), 126 deletions(-) diff --git a/cli-plugins/examples/helloworld/main.go b/cli-plugins/examples/helloworld/main.go index 597dc42b01..ba23f4954e 100644 --- a/cli-plugins/examples/helloworld/main.go +++ b/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") != "", }) } diff --git a/cli-plugins/manager/candidate_test.go b/cli-plugins/manager/candidate_test.go index 13f3317018..cbb85dc315 100644 --- a/cli-plugins/manager/candidate_test.go +++ b/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 != "" { diff --git a/cli-plugins/manager/manager.go b/cli-plugins/manager/manager.go index 57bd16b8e8..eaac482622 100644 --- a/cli-plugins/manager/manager.go +++ b/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...) diff --git a/cli-plugins/manager/metadata.go b/cli-plugins/manager/metadata.go index 74c226226f..5c80c1db86 100644 --- a/cli-plugins/manager/metadata.go +++ b/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"` } diff --git a/cli-plugins/manager/plugin.go b/cli-plugins/manager/plugin.go index fc3ad693cf..4b32a4fde6 100644 --- a/cli-plugins/manager/plugin.go +++ b/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 diff --git a/cli/command/cli.go b/cli/command/cli.go index 7b5048e98f..65ba4e8216 100644 --- a/cli/command/cli.go +++ b/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 } diff --git a/cli/command/cli_test.go b/cli/command/cli_test.go index 83f42e9df0..65cacdc61d 100644 --- a/cli/command/cli_test.go +++ b/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)) }) } } diff --git a/cli/command/stack/kubernetes/cli.go b/cli/command/stack/kubernetes/cli.go index 8f5baf6579..a531846809 100644 --- a/cli/command/stack/kubernetes/cli.go +++ b/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 { diff --git a/cli/command/stack/kubernetes/client.go b/cli/command/stack/kubernetes/client.go index fd516bb5d2..9a7dc7e3ee 100644 --- a/cli/command/stack/kubernetes/client.go +++ b/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 } diff --git a/cli/command/system/version.go b/cli/command/system/version.go index 0ab75bc9c8..c086822c88 100644 --- a/cli/command/system/version.go +++ b/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" diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 35ccb130b7..ff0fdf83ba 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -336,12 +336,11 @@ func hideSubcommandIf(subcmd *cobra.Command, condition func(string) bool, annota func hideUnsupportedFeatures(cmd *cobra.Command, details versionDetails) error { var ( - buildKitDisabled = func(_ string) bool { v, _ := command.BuildKitEnabled(details.ServerInfo()); return !v } - buildKitEnabled = func(_ string) bool { v, _ := command.BuildKitEnabled(details.ServerInfo()); return v } - notExperimental = func(_ string) bool { return !details.ServerInfo().HasExperimental } - notExperimentalCLI = func(_ string) bool { return !details.ClientInfo().HasExperimental } - notOSType = func(v string) bool { return v != details.ServerInfo().OSType } - versionOlderThan = func(v string) bool { return versions.LessThan(details.Client().ClientVersion(), v) } + buildKitDisabled = func(_ string) bool { v, _ := command.BuildKitEnabled(details.ServerInfo()); return !v } + buildKitEnabled = func(_ string) bool { v, _ := command.BuildKitEnabled(details.ServerInfo()); return v } + notExperimental = func(_ string) bool { return !details.ServerInfo().HasExperimental } + notOSType = func(v string) bool { return v != details.ServerInfo().OSType } + versionOlderThan = func(v string) bool { return versions.LessThan(details.Client().ClientVersion(), v) } ) cmd.Flags().VisitAll(func(f *pflag.Flag) { @@ -359,7 +358,6 @@ func hideUnsupportedFeatures(cmd *cobra.Command, details versionDetails) error { hideFlagIf(f, buildKitDisabled, "buildkit") hideFlagIf(f, buildKitEnabled, "no-buildkit") hideFlagIf(f, notExperimental, "experimental") - hideFlagIf(f, notExperimentalCLI, "experimentalCLI") hideFlagIf(f, notOSType, "ostype") hideFlagIf(f, versionOlderThan, "version") }) @@ -368,7 +366,6 @@ func hideUnsupportedFeatures(cmd *cobra.Command, details versionDetails) error { hideSubcommandIf(subcmd, buildKitDisabled, "buildkit") hideSubcommandIf(subcmd, buildKitEnabled, "no-buildkit") hideSubcommandIf(subcmd, notExperimental, "experimental") - hideSubcommandIf(subcmd, notExperimentalCLI, "experimentalCLI") hideSubcommandIf(subcmd, notOSType, "ostype") hideSubcommandIf(subcmd, versionOlderThan, "version") } @@ -417,9 +414,6 @@ func areFlagsSupported(cmd *cobra.Command, details versionDetails) error { if _, ok := f.Annotations["experimental"]; ok && !details.ServerInfo().HasExperimental { errs = append(errs, fmt.Sprintf(`"--%s" is only supported on a Docker daemon with experimental features enabled`, f.Name)) } - if _, ok := f.Annotations["experimentalCLI"]; ok && !details.ClientInfo().HasExperimental { - errs = append(errs, fmt.Sprintf(`"--%s" is only supported on a Docker cli with experimental cli features enabled`, f.Name)) - } // buildkit-specific flags are noop when buildkit is not enabled, so we do not add an error in that case }) if len(errs) > 0 { @@ -441,9 +435,6 @@ func areSubcommandsSupported(cmd *cobra.Command, details versionDetails) error { if _, ok := curr.Annotations["experimental"]; ok && !details.ServerInfo().HasExperimental { return fmt.Errorf("%s is only supported on a Docker daemon with experimental features enabled", cmd.CommandPath()) } - if _, ok := curr.Annotations["experimentalCLI"]; ok && !details.ClientInfo().HasExperimental { - return fmt.Errorf("%s is only supported on a Docker cli with experimental cli features enabled", cmd.CommandPath()) - } } return nil } diff --git a/docs/reference/commandline/cli.md b/docs/reference/commandline/cli.md index 498d32fb83..74edac265d 100644 --- a/docs/reference/commandline/cli.md +++ b/docs/reference/commandline/cli.md @@ -66,7 +66,6 @@ by the `docker` command line: * `DOCKER_API_VERSION` The API version to use (e.g. `1.19`) * `DOCKER_CONFIG` The location of your client configuration files. -* `DOCKER_CLI_EXPERIMENTAL` Enable experimental features for the cli (e.g. `enabled` or `disabled`) * `DOCKER_HOST` Daemon socket to connect to. * `DOCKER_STACK_ORCHESTRATOR` Configure the default orchestrator to use when using `docker stack` management commands. * `DOCKER_CONTENT_TRUST` When set Docker uses notary to sign and verify images. @@ -317,27 +316,8 @@ Following is a sample `config.json` file: ### Experimental features Experimental features provide early access to future product functionality. -These features are intended only for testing and feedback as they may change -between releases without warning or can be removed entirely from a future -release. - -> Experimental features must not be used in production environments. -{: .warning } - -To enable experimental features, edit the `config.json` file and set -`experimental` to `enabled`. The example below enables experimental features -in a `config.json` file that already enables a debug feature. - -```json -{ - "experimental": "enabled", - "debug": true -} -``` - -You can also enable experimental features from the Docker Desktop menu. See the -[Docker Desktop Getting Started page](https://docs.docker.com/docker-for-mac#experimental-features) -for more information. +These features are intended for testing and feedback, and they may change +between releases without warning or can be removed from a future release. ### Notary diff --git a/kubernetes/check.go b/kubernetes/check.go index 6a676fa1dc..41ba631598 100644 --- a/kubernetes/check.go +++ b/kubernetes/check.go @@ -24,18 +24,18 @@ const ( ) // GetStackAPIVersion returns the most appropriate stack API version installed. -func GetStackAPIVersion(serverGroups discovery.ServerGroupsInterface, experimental bool) (StackVersion, error) { +func GetStackAPIVersion(serverGroups discovery.ServerGroupsInterface) (StackVersion, error) { groups, err := serverGroups.ServerGroups() if err != nil { return "", err } - return getAPIVersion(groups, experimental) + return getAPIVersion(groups) } -func getAPIVersion(groups *metav1.APIGroupList, experimental bool) (StackVersion, error) { +func getAPIVersion(groups *metav1.APIGroupList) (StackVersion, error) { switch { - case experimental && findVersion(apiv1alpha3.SchemeGroupVersion, groups.Groups): + case findVersion(apiv1alpha3.SchemeGroupVersion, groups.Groups): return StackAPIV1Alpha3, nil case findVersion(apiv1beta2.SchemeGroupVersion, groups.Groups): return StackAPIV1Beta2, nil diff --git a/kubernetes/check_test.go b/kubernetes/check_test.go index ec1ac6d36e..b642545ad5 100644 --- a/kubernetes/check_test.go +++ b/kubernetes/check_test.go @@ -12,20 +12,18 @@ func TestGetStackAPIVersion(t *testing.T) { var tests = []struct { description string groups *metav1.APIGroupList - experimental bool err bool expectedStack StackVersion }{ - {"no stack api", makeGroups(), false, true, ""}, - {"v1beta1", makeGroups(groupVersion{"compose.docker.com", []string{"v1beta1"}}), false, false, StackAPIV1Beta1}, - {"v1beta2", makeGroups(groupVersion{"compose.docker.com", []string{"v1beta2"}}), false, false, StackAPIV1Beta2}, - {"most recent has precedence", makeGroups(groupVersion{"compose.docker.com", []string{"v1beta1", "v1beta2"}}), false, false, StackAPIV1Beta2}, - {"most recent has precedence", makeGroups(groupVersion{"compose.docker.com", []string{"v1beta1", "v1beta2", "v1alpha3"}}), false, false, StackAPIV1Beta2}, - {"most recent has precedence", makeGroups(groupVersion{"compose.docker.com", []string{"v1beta1", "v1beta2", "v1alpha3"}}), true, false, StackAPIV1Alpha3}, + {"no stack api", makeGroups(), true, ""}, + {"v1beta1", makeGroups(groupVersion{"compose.docker.com", []string{"v1beta1"}}), false, StackAPIV1Beta1}, + {"v1beta2", makeGroups(groupVersion{"compose.docker.com", []string{"v1beta2"}}), false, StackAPIV1Beta2}, + {"most recent has precedence", makeGroups(groupVersion{"compose.docker.com", []string{"v1beta1", "v1beta2"}}), false, StackAPIV1Beta2}, + {"most recent has precedence", makeGroups(groupVersion{"compose.docker.com", []string{"v1beta1", "v1beta2", "v1alpha3"}}), false, StackAPIV1Alpha3}, } for _, test := range tests { - version, err := getAPIVersion(test.groups, test.experimental) + version, err := getAPIVersion(test.groups) if test.err { assert.ErrorContains(t, err, "") } else {