Merge pull request #33438 from vdemeester/remove-cli-flags

Remove `cli/flags` package

cli/flags/configdir.go → cli/config/configdir.go

@@ -1,4 +1,4 @@
-package flags
+package config
 
 import (
 	"os"
@@ -12,9 +12,9 @@ var (
 	configFileDir = ".docker"
 )
 
-// ConfigurationDir returns the path to the configuration directory as specified by the DOCKER_CONFIG environment variable.
+// Dir returns the path to the configuration directory as specified by the DOCKER_CONFIG environment variable.
 // TODO: this was copied from cli/config/configfile and should be removed once cmd/dockerd moves
-func ConfigurationDir() string {
+func Dir() string {
 	return configDir
 }
 

cli/flags/client.go

@@ -1,13 +0,0 @@
-package flags
-
-// ClientOptions are the options used to configure the client cli
-type ClientOptions struct {
-	Common    *CommonOptions
-	ConfigDir string
-	Version   bool
-}
-
-// NewClientOptions returns a new ClientOptions
-func NewClientOptions() *ClientOptions {
-	return &ClientOptions{Common: NewCommonOptions()}
-}

cmd/dockerd/daemon.go

@@ -27,7 +27,6 @@ import (
 	systemrouter "github.com/docker/docker/api/server/router/system"
 	"github.com/docker/docker/api/server/router/volume"
 	"github.com/docker/docker/cli/debug"
-	cliflags "github.com/docker/docker/cli/flags"
 	"github.com/docker/docker/daemon"
 	"github.com/docker/docker/daemon/cluster"
 	"github.com/docker/docker/daemon/config"
@@ -65,14 +64,14 @@ func NewDaemonCli() *DaemonCli {
 	return &DaemonCli{}
 }
 
-func (cli *DaemonCli) start(opts daemonOptions) (err error) {
+func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
 	stopc := make(chan bool)
 	defer close(stopc)
 
 	// warn from uuid package when running the daemon
 	uuid.Loggerf = logrus.Warnf
 
-	opts.common.SetDefaultOptions(opts.flags)
+	opts.SetDefaultOptions(opts.flags)
 
 	if cli.Config, err = loadDaemonCliConfig(opts); err != nil {
 		return err
@@ -358,20 +357,20 @@ func shutdownDaemon(d *daemon.Daemon) {
 	}
 }
 
-func loadDaemonCliConfig(opts daemonOptions) (*config.Config, error) {
+func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) {
 	conf := opts.daemonConfig
 	flags := opts.flags
-	conf.Debug = opts.common.Debug
-	conf.Hosts = opts.common.Hosts
-	conf.LogLevel = opts.common.LogLevel
-	conf.TLS = opts.common.TLS
-	conf.TLSVerify = opts.common.TLSVerify
+	conf.Debug = opts.Debug
+	conf.Hosts = opts.Hosts
+	conf.LogLevel = opts.LogLevel
+	conf.TLS = opts.TLS
+	conf.TLSVerify = opts.TLSVerify
 	conf.CommonTLSOptions = config.CommonTLSOptions{}
 
-	if opts.common.TLSOptions != nil {
-		conf.CommonTLSOptions.CAFile = opts.common.TLSOptions.CAFile
-		conf.CommonTLSOptions.CertFile = opts.common.TLSOptions.CertFile
-		conf.CommonTLSOptions.KeyFile = opts.common.TLSOptions.KeyFile
+	if opts.TLSOptions != nil {
+		conf.CommonTLSOptions.CAFile = opts.TLSOptions.CAFile
+		conf.CommonTLSOptions.CertFile = opts.TLSOptions.CertFile
+		conf.CommonTLSOptions.KeyFile = opts.TLSOptions.KeyFile
 	}
 
 	if conf.TrustKeyPath == "" {
@@ -425,12 +424,12 @@ func loadDaemonCliConfig(opts daemonOptions) (*config.Config, error) {
 
 	// Regardless of whether the user sets it to true or false, if they
 	// specify TLSVerify at all then we need to turn on TLS
-	if conf.IsValueSet(cliflags.FlagTLSVerify) {
+	if conf.IsValueSet(FlagTLSVerify) {
 		conf.TLS = true
 	}
 
 	// ensure that the log level is the one set after merging configurations
-	cliflags.SetLogLevel(conf.LogLevel)
+	setLogLevel(conf.LogLevel)
 
 	return conf, nil
 }

cmd/dockerd/daemon_test.go

@@ -4,7 +4,6 @@ import (
 	"testing"
 
 	"github.com/Sirupsen/logrus"
-	cliflags "github.com/docker/docker/cli/flags"
 	"github.com/docker/docker/daemon/config"
 	"github.com/docker/docker/pkg/testutil"
 	"github.com/docker/docker/pkg/testutil/tempfile"
@@ -13,13 +12,10 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func defaultOptions(configFile string) daemonOptions {
-	opts := daemonOptions{
-		daemonConfig: &config.Config{},
-		flags:        &pflag.FlagSet{},
-		common:       cliflags.NewCommonOptions(),
-	}
-	opts.common.InstallFlags(opts.flags)
+func defaultOptions(configFile string) *daemonOptions {
+	opts := newDaemonOptions(&config.Config{})
+	opts.flags = &pflag.FlagSet{}
+	opts.InstallFlags(opts.flags)
 	installConfigFlags(opts.daemonConfig, opts.flags)
 	opts.flags.StringVar(&opts.configFile, "config-file", defaultDaemonConfigFile, "")
 	opts.configFile = configFile
@@ -28,7 +24,7 @@ func defaultOptions(configFile string) daemonOptions {
 
 func TestLoadDaemonCliConfigWithoutOverriding(t *testing.T) {
 	opts := defaultOptions("")
-	opts.common.Debug = true
+	opts.Debug = true
 
 	loadedConfig, err := loadDaemonCliConfig(opts)
 	require.NoError(t, err)
@@ -40,8 +36,8 @@ func TestLoadDaemonCliConfigWithoutOverriding(t *testing.T) {
 
 func TestLoadDaemonCliConfigWithTLS(t *testing.T) {
 	opts := defaultOptions("")
-	opts.common.TLSOptions.CAFile = "/tmp/ca.pem"
-	opts.common.TLS = true
+	opts.TLSOptions.CAFile = "/tmp/ca.pem"
+	opts.TLS = true
 
 	loadedConfig, err := loadDaemonCliConfig(opts)
 	require.NoError(t, err)
@@ -70,7 +66,7 @@ func TestLoadDaemonCliConfigWithTLSVerify(t *testing.T) {
 	defer tempFile.Remove()
 
 	opts := defaultOptions(tempFile.Name())
-	opts.common.TLSOptions.CAFile = "/tmp/ca.pem"
+	opts.TLSOptions.CAFile = "/tmp/ca.pem"
 
 	loadedConfig, err := loadDaemonCliConfig(opts)
 	require.NoError(t, err)
@@ -83,7 +79,7 @@ func TestLoadDaemonCliConfigWithExplicitTLSVerifyFalse(t *testing.T) {
 	defer tempFile.Remove()
 
 	opts := defaultOptions(tempFile.Name())
-	opts.common.TLSOptions.CAFile = "/tmp/ca.pem"
+	opts.TLSOptions.CAFile = "/tmp/ca.pem"
 
 	loadedConfig, err := loadDaemonCliConfig(opts)
 	require.NoError(t, err)
@@ -96,7 +92,7 @@ func TestLoadDaemonCliConfigWithoutTLSVerify(t *testing.T) {
 	defer tempFile.Remove()
 
 	opts := defaultOptions(tempFile.Name())
-	opts.common.TLSOptions.CAFile = "/tmp/ca.pem"
+	opts.TLSOptions.CAFile = "/tmp/ca.pem"
 
 	loadedConfig, err := loadDaemonCliConfig(opts)
 	require.NoError(t, err)

cmd/dockerd/daemon_unix_test.go

@@ -20,8 +20,8 @@ func TestLoadDaemonCliConfigWithDaemonFlags(t *testing.T) {
 	defer tempFile.Remove()
 
 	opts := defaultOptions(tempFile.Name())
-	opts.common.Debug = true
-	opts.common.LogLevel = "info"
+	opts.Debug = true
+	opts.LogLevel = "info"
 	assert.NoError(t, opts.flags.Set("selinux-enabled", "true"))
 
 	loadedConfig, err := loadDaemonCliConfig(opts)

cmd/dockerd/docker.go

@@ -8,28 +8,15 @@ import (
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/cli"
-	cliflags "github.com/docker/docker/cli/flags"
 	"github.com/docker/docker/daemon/config"
 	"github.com/docker/docker/dockerversion"
 	"github.com/docker/docker/pkg/reexec"
 	"github.com/docker/docker/pkg/term"
 	"github.com/spf13/cobra"
-	"github.com/spf13/pflag"
 )
 
-type daemonOptions struct {
-	version      bool
-	configFile   string
-	daemonConfig *config.Config
-	common       *cliflags.CommonOptions
-	flags        *pflag.FlagSet
-}
-
 func newDaemonCommand() *cobra.Command {
-	opts := daemonOptions{
-		daemonConfig: config.New(),
-		common:       cliflags.NewCommonOptions(),
-	}
+	opts := newDaemonOptions(config.New())
 
 	cmd := &cobra.Command{
 		Use:           "dockerd [OPTIONS]",
@@ -47,14 +34,14 @@ func newDaemonCommand() *cobra.Command {
 	flags := cmd.Flags()
 	flags.BoolVarP(&opts.version, "version", "v", false, "Print version information and quit")
 	flags.StringVar(&opts.configFile, "config-file", defaultDaemonConfigFile, "Daemon configuration file")
-	opts.common.InstallFlags(flags)
+	opts.InstallFlags(flags)
 	installConfigFlags(opts.daemonConfig, flags)
 	installServiceFlags(flags)
 
 	return cmd
 }
 
-func runDaemon(opts daemonOptions) error {
+func runDaemon(opts *daemonOptions) error {
 	if opts.version {
 		showVersion()
 		return nil

cli/flags/common.go → cmd/dockerd/options.go

@@ -1,4 +1,4 @@
-package flags
+package main
 
 import (
 	"fmt"
@@ -6,6 +6,8 @@ import (
 	"path/filepath"
 
 	"github.com/Sirupsen/logrus"
+	cliconfig "github.com/docker/docker/cli/config"
+	"github.com/docker/docker/daemon/config"
 	"github.com/docker/docker/opts"
 	"github.com/docker/go-connections/tlsconfig"
 	"github.com/spf13/pflag"
@@ -27,64 +29,69 @@ var (
 	dockerTLSVerify = os.Getenv("DOCKER_TLS_VERIFY") != ""
 )
 
-// CommonOptions are options common to both the client and the daemon.
-type CommonOptions struct {
-	Debug      bool
-	Hosts      []string
-	LogLevel   string
-	TLS        bool
-	TLSVerify  bool
-	TLSOptions *tlsconfig.Options
+type daemonOptions struct {
+	version      bool
+	configFile   string
+	daemonConfig *config.Config
+	flags        *pflag.FlagSet
+	Debug        bool
+	Hosts        []string
+	LogLevel     string
+	TLS          bool
+	TLSVerify    bool
+	TLSOptions   *tlsconfig.Options
 }
 
-// NewCommonOptions returns a new CommonOptions
-func NewCommonOptions() *CommonOptions {
-	return &CommonOptions{}
+// newDaemonOptions returns a new daemonFlags
+func newDaemonOptions(config *config.Config) *daemonOptions {
+	return &daemonOptions{
+		daemonConfig: config,
+	}
 }
 
 // InstallFlags adds flags for the common options on the FlagSet
-func (commonOpts *CommonOptions) InstallFlags(flags *pflag.FlagSet) {
+func (o *daemonOptions) InstallFlags(flags *pflag.FlagSet) {
 	if dockerCertPath == "" {
-		dockerCertPath = ConfigurationDir()
+		dockerCertPath = cliconfig.Dir()
 	}
 
-	flags.BoolVarP(&commonOpts.Debug, "debug", "D", false, "Enable debug mode")
-	flags.StringVarP(&commonOpts.LogLevel, "log-level", "l", "info", `Set the logging level ("debug"|"info"|"warn"|"error"|"fatal")`)
-	flags.BoolVar(&commonOpts.TLS, "tls", false, "Use TLS; implied by --tlsverify")
-	flags.BoolVar(&commonOpts.TLSVerify, FlagTLSVerify, dockerTLSVerify, "Use TLS and verify the remote")
+	flags.BoolVarP(&o.Debug, "debug", "D", false, "Enable debug mode")
+	flags.StringVarP(&o.LogLevel, "log-level", "l", "info", `Set the logging level ("debug"|"info"|"warn"|"error"|"fatal")`)
+	flags.BoolVar(&o.TLS, "tls", false, "Use TLS; implied by --tlsverify")
+	flags.BoolVar(&o.TLSVerify, FlagTLSVerify, dockerTLSVerify, "Use TLS and verify the remote")
 
 	// TODO use flag flags.String("identity"}, "i", "", "Path to libtrust key file")
 
-	commonOpts.TLSOptions = &tlsconfig.Options{
+	o.TLSOptions = &tlsconfig.Options{
 		CAFile:   filepath.Join(dockerCertPath, DefaultCaFile),
 		CertFile: filepath.Join(dockerCertPath, DefaultCertFile),
 		KeyFile:  filepath.Join(dockerCertPath, DefaultKeyFile),
 	}
-	tlsOptions := commonOpts.TLSOptions
+	tlsOptions := o.TLSOptions
 	flags.Var(opts.NewQuotedString(&tlsOptions.CAFile), "tlscacert", "Trust certs signed only by this CA")
 	flags.Var(opts.NewQuotedString(&tlsOptions.CertFile), "tlscert", "Path to TLS certificate file")
 	flags.Var(opts.NewQuotedString(&tlsOptions.KeyFile), "tlskey", "Path to TLS key file")
 
-	hostOpt := opts.NewNamedListOptsRef("hosts", &commonOpts.Hosts, opts.ValidateHost)
+	hostOpt := opts.NewNamedListOptsRef("hosts", &o.Hosts, opts.ValidateHost)
 	flags.VarP(hostOpt, "host", "H", "Daemon socket(s) to connect to")
 }
 
 // SetDefaultOptions sets default values for options after flag parsing is
 // complete
-func (commonOpts *CommonOptions) SetDefaultOptions(flags *pflag.FlagSet) {
+func (o *daemonOptions) SetDefaultOptions(flags *pflag.FlagSet) {
 	// Regardless of whether the user sets it to true or false, if they
 	// specify --tlsverify at all then we need to turn on TLS
 	// TLSVerify can be true even if not set due to DOCKER_TLS_VERIFY env var, so we need
 	// to check that here as well
-	if flags.Changed(FlagTLSVerify) || commonOpts.TLSVerify {
-		commonOpts.TLS = true
+	if flags.Changed(FlagTLSVerify) || o.TLSVerify {
+		o.TLS = true
 	}
 
-	if !commonOpts.TLS {
-		commonOpts.TLSOptions = nil
+	if !o.TLS {
+		o.TLSOptions = nil
 	} else {
-		tlsOptions := commonOpts.TLSOptions
-		tlsOptions.InsecureSkipVerify = !commonOpts.TLSVerify
+		tlsOptions := o.TLSOptions
+		tlsOptions.InsecureSkipVerify = !o.TLSVerify
 
 		// Reset CertFile and KeyFile to empty string if the user did not specify
 		// the respective flags and the respective default files were not found.
@@ -101,8 +108,8 @@ func (commonOpts *CommonOptions) SetDefaultOptions(flags *pflag.FlagSet) {
 	}
 }
 
-// SetLogLevel sets the logrus logging level
-func SetLogLevel(logLevel string) {
+// setLogLevel sets the logrus logging level
+func setLogLevel(logLevel string) {
 	if logLevel != "" {
 		lvl, err := logrus.ParseLevel(logLevel)
 		if err != nil {

cli/flags/common_test.go → cmd/dockerd/options_test.go

@@ -1,16 +1,18 @@
-package flags
+package main
 
 import (
 	"path/filepath"
 	"testing"
 
+	cliconfig "github.com/docker/docker/cli/config"
+	"github.com/docker/docker/daemon/config"
 	"github.com/spf13/pflag"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestCommonOptionsInstallFlags(t *testing.T) {
 	flags := pflag.NewFlagSet("testing", pflag.ContinueOnError)
-	opts := NewCommonOptions()
+	opts := newDaemonOptions(&config.Config{})
 	opts.InstallFlags(flags)
 
 	err := flags.Parse([]string{
@@ -25,12 +27,12 @@ func TestCommonOptionsInstallFlags(t *testing.T) {
 }
 
 func defaultPath(filename string) string {
-	return filepath.Join(ConfigurationDir(), filename)
+	return filepath.Join(cliconfig.Dir(), filename)
 }
 
 func TestCommonOptionsInstallFlagsWithDefaults(t *testing.T) {
 	flags := pflag.NewFlagSet("testing", pflag.ContinueOnError)
-	opts := NewCommonOptions()
+	opts := newDaemonOptions(&config.Config{})
 	opts.InstallFlags(flags)
 
 	err := flags.Parse([]string{})

integration-cli/check_test.go

@@ -12,7 +12,7 @@ import (
 	"testing"
 
 	"github.com/docker/docker/api/types/swarm"
-	"github.com/docker/docker/cli/flags"
+	"github.com/docker/docker/cli/config"
 	"github.com/docker/docker/integration-cli/cli"
 	"github.com/docker/docker/integration-cli/cli/build/fakestorage"
 	"github.com/docker/docker/integration-cli/daemon"
@@ -406,7 +406,7 @@ func (s *DockerTrustSuite) TearDownTest(c *check.C) {
 	}
 
 	// Remove trusted keys and metadata after test
-	os.RemoveAll(filepath.Join(flags.ConfigurationDir(), "trust"))
+	os.RemoveAll(filepath.Join(config.Dir(), "trust"))
 	s.ds.TearDownTest(c)
 }
 

integration-cli/docker_cli_push_test.go

@@ -12,7 +12,7 @@ import (
 	"sync"
 
 	"github.com/docker/distribution/reference"
-	"github.com/docker/docker/cli/flags"
+	"github.com/docker/docker/cli/config"
 	"github.com/docker/docker/integration-cli/checker"
 	"github.com/docker/docker/integration-cli/cli"
 	"github.com/docker/docker/integration-cli/cli/build"
@@ -294,7 +294,7 @@ func (s *DockerTrustSuite) TestTrustedPush(c *check.C) {
 	})
 
 	// Assert that we rotated the snapshot key to the server by checking our local keystore
-	contents, err := ioutil.ReadDir(filepath.Join(flags.ConfigurationDir(), "trust/private/tuf_keys", privateRegistryURL, "dockerclitrusted/pushtest"))
+	contents, err := ioutil.ReadDir(filepath.Join(config.Dir(), "trust/private/tuf_keys", privateRegistryURL, "dockerclitrusted/pushtest"))
 	c.Assert(err, check.IsNil, check.Commentf("Unable to read local tuf key files"))
 	// Check that we only have 1 key (targets key)
 	c.Assert(contents, checker.HasLen, 1)
@@ -399,7 +399,7 @@ func (s *DockerTrustSuite) TestTrustedPushWithReleasesDelegationOnly(c *check.C)
 	s.assertTargetNotInRoles(c, repoName, "latest", "targets")
 
 	// Try pull after push
-	os.RemoveAll(filepath.Join(flags.ConfigurationDir(), "trust"))
+	os.RemoveAll(filepath.Join(config.Dir(), "trust"))
 
 	cli.Docker(cli.Args("pull", targetName), trustedCmd).Assert(c, icmd.Expected{
 		Out: "Status: Image is up to date",
@@ -436,7 +436,7 @@ func (s *DockerTrustSuite) TestTrustedPushSignsAllFirstLevelRolesWeHaveKeysFor(c
 	s.assertTargetNotInRoles(c, repoName, "latest", "targets")
 
 	// Try pull after push
-	os.RemoveAll(filepath.Join(flags.ConfigurationDir(), "trust"))
+	os.RemoveAll(filepath.Join(config.Dir(), "trust"))
 
 	// pull should fail because none of these are the releases role
 	cli.Docker(cli.Args("pull", targetName), trustedCmd).Assert(c, icmd.Expected{
@@ -472,7 +472,7 @@ func (s *DockerTrustSuite) TestTrustedPushSignsForRolesWithKeysAndValidPaths(c *
 	s.assertTargetNotInRoles(c, repoName, "latest", "targets")
 
 	// Try pull after push
-	os.RemoveAll(filepath.Join(flags.ConfigurationDir(), "trust"))
+	os.RemoveAll(filepath.Join(config.Dir(), "trust"))
 
 	// pull should fail because none of these are the releases role
 	cli.Docker(cli.Args("pull", targetName), trustedCmd).Assert(c, icmd.Expected{

integration-cli/trust_server_test.go

@@ -11,7 +11,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/docker/docker/cli/flags"
+	cliconfig "github.com/docker/docker/cli/config"
 	"github.com/docker/docker/integration-cli/checker"
 	"github.com/docker/docker/integration-cli/cli"
 	icmd "github.com/docker/docker/pkg/testutil/cmd"
@@ -108,7 +108,7 @@ func newTestNotary(c *check.C) (*testNotary, error) {
 		"skipTLSVerify": true
 	}
 }`
-	if _, err = fmt.Fprintf(clientConfig, template, filepath.Join(flags.ConfigurationDir(), "trust"), notaryURL); err != nil {
+	if _, err = fmt.Fprintf(clientConfig, template, filepath.Join(cliconfig.Dir(), "trust"), notaryURL); err != nil {
 		os.RemoveAll(tmp)
 		return nil, err
 	}