From f1b116179f2a95d0ea45780dfb8be51c2825b9c0 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Tue, 2 Jan 2018 23:56:07 +0100 Subject: [PATCH] Fix PR comments - More strict on orchestrator flag - Make orchestrator flag more explicit as experimental - Add experimentalCLI annotation on kubernetes flags - Better kubeconfig error message - Prefix service name with stackname in ps and services stack subcommands - Fix yaml documentation - Fix code coverage ignoring generated code Signed-off-by: Silvin Lubecki --- cli/command/cli.go | 7 ++-- cli/command/cli_test.go | 4 +-- cli/command/orchestrator.go | 9 +++-- cli/command/stack/cmd.go | 2 ++ cli/command/stack/deploy.go | 2 +- cli/command/stack/kubernetes/cli.go | 3 +- cli/command/stack/kubernetes/conversion.go | 6 +++- cli/command/stack/kubernetes/deploy.go | 26 +++++++------- cli/command/stack/kubernetes/ps.go | 6 ++-- cli/command/stack/list.go | 2 +- cli/command/stack/ps.go | 2 +- cli/command/stack/remove.go | 2 +- cli/command/stack/services.go | 2 +- cli/flags/common.go | 3 +- cmd/docker/docker.go | 6 ++-- codecov.yml | 3 ++ docs/yaml/yaml.go | 40 +++++++++++++++++----- e2e/compose-env.yaml | 2 -- e2e/stack/remove_test.go | 5 +-- kubernetes/README.md | 3 +- 20 files changed, 83 insertions(+), 52 deletions(-) diff --git a/cli/command/cli.go b/cli/command/cli.go index 57379ce749..5d903ccead 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -140,7 +140,6 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions) error { cli.clientInfo = ClientInfo{ DefaultVersion: cli.client.ClientVersion(), HasExperimental: hasExperimental, - HasKubernetes: hasExperimental && orchestrator == OrchestratorKubernetes, Orchestrator: orchestrator, } cli.initializeFromClient() @@ -206,11 +205,15 @@ type ServerInfo struct { // ClientInfo stores details about the supported features of the client type ClientInfo struct { HasExperimental bool - HasKubernetes bool DefaultVersion string Orchestrator Orchestrator } +// HasKubernetes checks if kubernetes orchestrator is enabled +func (c ClientInfo) HasKubernetes() bool { + return c.HasExperimental && c.Orchestrator == OrchestratorKubernetes +} + // NewDockerCli returns a DockerCli instance with IO output and error streams set by in, out and err. func NewDockerCli(in io.ReadCloser, out, err io.Writer) *DockerCli { return &DockerCli{in: NewInStream(in), out: NewOutStream(out), err: err} diff --git a/cli/command/cli_test.go b/cli/command/cli_test.go index 37047551e5..9a440ef138 100644 --- a/cli/command/cli_test.go +++ b/cli/command/cli_test.go @@ -171,7 +171,7 @@ func TestExperimentalCLI(t *testing.T) { } func TestOrchestratorSwitch(t *testing.T) { - defaultVersion := "v1.55" + defaultVersion := "v0.00" var testcases = []struct { doc string @@ -268,7 +268,7 @@ func TestOrchestratorSwitch(t *testing.T) { } err := cli.Initialize(options) assert.NoError(t, err) - assert.Equal(t, testcase.expectedKubernetes, cli.ClientInfo().HasKubernetes) + assert.Equal(t, testcase.expectedKubernetes, cli.ClientInfo().HasKubernetes()) assert.Equal(t, testcase.expectedOrchestrator, string(cli.ClientInfo().Orchestrator)) }) } diff --git a/cli/command/orchestrator.go b/cli/command/orchestrator.go index 520f030196..4efe623c29 100644 --- a/cli/command/orchestrator.go +++ b/cli/command/orchestrator.go @@ -3,7 +3,6 @@ package command import ( "fmt" "os" - "strings" ) // Orchestrator type acts as an enum describing supported orchestrators. @@ -16,12 +15,12 @@ const ( OrchestratorSwarm = Orchestrator("swarm") orchestratorUnset = Orchestrator("unset") - defaultOrchestrator = OrchestratorSwarm - dockerOrchestrator = "DOCKER_ORCHESTRATOR" + defaultOrchestrator = OrchestratorSwarm + envVarDockerOrchestrator = "DOCKER_ORCHESTRATOR" ) func normalize(flag string) Orchestrator { - switch strings.ToLower(flag) { + switch flag { case "kubernetes", "k8s": return OrchestratorKubernetes case "swarm", "swarmkit": @@ -43,7 +42,7 @@ func GetOrchestrator(isExperimental bool, flagValue, value string) Orchestrator return o } // Check environment variable - env := os.Getenv(dockerOrchestrator) + env := os.Getenv(envVarDockerOrchestrator) if o := normalize(env); o != orchestratorUnset { return o } diff --git a/cli/command/stack/cmd.go b/cli/command/stack/cmd.go index 7186c28f66..0616ab1689 100644 --- a/cli/command/stack/cmd.go +++ b/cli/command/stack/cmd.go @@ -25,8 +25,10 @@ func NewStackCommand(dockerCli command.Cli) *cobra.Command { flags := cmd.PersistentFlags() flags.String("namespace", "default", "Kubernetes namespace to use") flags.SetAnnotation("namespace", "kubernetes", nil) + flags.SetAnnotation("namespace", "experimentalCLI", nil) flags.String("kubeconfig", "", "Kubernetes config file") flags.SetAnnotation("kubeconfig", "kubernetes", nil) + flags.SetAnnotation("kubeconfig", "experimentalCLI", nil) return cmd } diff --git a/cli/command/stack/deploy.go b/cli/command/stack/deploy.go index f9866716a8..d01add7b98 100644 --- a/cli/command/stack/deploy.go +++ b/cli/command/stack/deploy.go @@ -19,7 +19,7 @@ func newDeployCommand(dockerCli command.Cli) *cobra.Command { Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts.Namespace = args[0] - if dockerCli.ClientInfo().HasKubernetes { + if dockerCli.ClientInfo().HasKubernetes() { kli, err := kubernetes.WrapCli(dockerCli, cmd) if err != nil { return err diff --git a/cli/command/stack/kubernetes/cli.go b/cli/command/stack/kubernetes/cli.go index 44790d7819..90a4b71c78 100644 --- a/cli/command/stack/kubernetes/cli.go +++ b/cli/command/stack/kubernetes/cli.go @@ -1,6 +1,7 @@ package kubernetes import ( + "fmt" "os" "path/filepath" @@ -49,7 +50,7 @@ func WrapCli(dockerCli command.Cli, cmd *cobra.Command) (*KubeCli, error) { config, err := clientcmd.BuildConfigFromFlags("", kubeConfig) if err != nil { - return nil, err + return nil, fmt.Errorf("Failed to load kubernetes configuration file '%s'", kubeConfig) } cli.kubeConfig = config diff --git a/cli/command/stack/kubernetes/conversion.go b/cli/command/stack/kubernetes/conversion.go index 9c5eccd820..a62d28a34c 100644 --- a/cli/command/stack/kubernetes/conversion.go +++ b/cli/command/stack/kubernetes/conversion.go @@ -126,12 +126,16 @@ func replicasToServices(replicas *appsv1beta2.ReplicaSetList, services *apiv1.Se if !ok { return nil, nil, fmt.Errorf("could not find service '%s'", r.Labels[labels.ForServiceName]) } + stack, ok := service.Labels[labels.ForStackName] + if ok { + stack += "_" + } uid := string(service.UID) s := swarm.Service{ ID: uid, Spec: swarm.ServiceSpec{ Annotations: swarm.Annotations{ - Name: service.Name, + Name: stack + service.Name, }, TaskTemplate: swarm.TaskSpec{ ContainerSpec: &swarm.ContainerSpec{ diff --git a/cli/command/stack/kubernetes/deploy.go b/cli/command/stack/kubernetes/deploy.go index da4679d2e7..b269445b97 100644 --- a/cli/command/stack/kubernetes/deploy.go +++ b/cli/command/stack/kubernetes/deploy.go @@ -20,7 +20,7 @@ func RunDeploy(dockerCli *KubeCli, opts options.Deploy) error { return errors.Errorf("Please specify a Compose file (with --compose-file).") } // Initialize clients - stackInterface, err := dockerCli.stacks() + stacks, err := dockerCli.stacks() if err != nil { return err } @@ -28,12 +28,12 @@ func RunDeploy(dockerCli *KubeCli, opts options.Deploy) error { if err != nil { return err } - configMapInterface := composeClient.ConfigMaps() - secretInterface := composeClient.Secrets() - serviceInterface := composeClient.Services() - podInterface := composeClient.Pods() + configMaps := composeClient.ConfigMaps() + secrets := composeClient.Secrets() + services := composeClient.Services() + pods := composeClient.Pods() watcher := DeployWatcher{ - Pods: podInterface, + Pods: pods, } // Parse the compose file @@ -43,28 +43,28 @@ func RunDeploy(dockerCli *KubeCli, opts options.Deploy) error { } // FIXME(vdemeester) handle warnings server-side - if err = IsColliding(serviceInterface, stack, cfg); err != nil { + if err = IsColliding(services, stack, cfg); err != nil { return err } - if err = createFileBasedConfigMaps(stack.Name, cfg.Configs, configMapInterface); err != nil { + if err = createFileBasedConfigMaps(stack.Name, cfg.Configs, configMaps); err != nil { return err } - if err = createFileBasedSecrets(stack.Name, cfg.Secrets, secretInterface); err != nil { + if err = createFileBasedSecrets(stack.Name, cfg.Secrets, secrets); err != nil { return err } - if in, err := stackInterface.Get(stack.Name, metav1.GetOptions{}); err == nil { + if in, err := stacks.Get(stack.Name, metav1.GetOptions{}); err == nil { in.Spec = stack.Spec - if _, err = stackInterface.Update(in); err != nil { + if _, err = stacks.Update(in); err != nil { return err } fmt.Printf("Stack %s was updated\n", stack.Name) } else { - if _, err = stackInterface.Create(stack); err != nil { + if _, err = stacks.Create(stack); err != nil { return err } @@ -76,7 +76,7 @@ func RunDeploy(dockerCli *KubeCli, opts options.Deploy) error { <-watcher.Watch(stack, serviceNames(cfg)) fmt.Fprintf(cmdOut, "Stack %s is stable and running\n\n", stack.Name) - // fmt.Fprintf(cmdOut, "Read the logs with:\n $ %s stack logs %s\n", filepath.Base(os.Args[0]), stack.Name) + // TODO: fmt.Fprintf(cmdOut, "Read the logs with:\n $ %s stack logs %s\n", filepath.Base(os.Args[0]), stack.Name) return nil } diff --git a/cli/command/stack/kubernetes/ps.go b/cli/command/stack/kubernetes/ps.go index d33acabcbe..00d05febb7 100644 --- a/cli/command/stack/kubernetes/ps.go +++ b/cli/command/stack/kubernetes/ps.go @@ -55,12 +55,12 @@ func RunPS(dockerCli *KubeCli, options options.PS) error { for i, pod := range pods { tasks[i] = podToTask(pod) } - return print(dockerCli, tasks, pods, nodeResolver, !options.NoTrunc, options.Quiet, format) + return print(dockerCli, namespace, tasks, pods, nodeResolver, !options.NoTrunc, options.Quiet, format) } type idResolver func(name string) (string, error) -func print(dockerCli command.Cli, tasks []swarm.Task, pods []apiv1.Pod, nodeResolver idResolver, trunc, quiet bool, format string) error { +func print(dockerCli command.Cli, namespace string, tasks []swarm.Task, pods []apiv1.Pod, nodeResolver idResolver, trunc, quiet bool, format string) error { sort.Stable(tasksBySlot(tasks)) names := map[string]string{} @@ -78,7 +78,7 @@ func print(dockerCli command.Cli, tasks []swarm.Task, pods []apiv1.Pod, nodeReso return err } - names[task.ID] = pods[i].Name + names[task.ID] = fmt.Sprintf("%s_%s", namespace, pods[i].Name) nodes[task.ID] = nodeValue } diff --git a/cli/command/stack/list.go b/cli/command/stack/list.go index 4db83765fe..2cb10c1716 100644 --- a/cli/command/stack/list.go +++ b/cli/command/stack/list.go @@ -18,7 +18,7 @@ func newListCommand(dockerCli command.Cli) *cobra.Command { Short: "List stacks", Args: cli.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - if dockerCli.ClientInfo().HasKubernetes { + if dockerCli.ClientInfo().HasKubernetes() { kli, err := kubernetes.WrapCli(dockerCli, cmd) if err != nil { return err diff --git a/cli/command/stack/ps.go b/cli/command/stack/ps.go index cd8c32a28e..8e835201a2 100644 --- a/cli/command/stack/ps.go +++ b/cli/command/stack/ps.go @@ -19,7 +19,7 @@ func newPsCommand(dockerCli command.Cli) *cobra.Command { Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts.Namespace = args[0] - if dockerCli.ClientInfo().HasKubernetes { + if dockerCli.ClientInfo().HasKubernetes() { kli, err := kubernetes.WrapCli(dockerCli, cmd) if err != nil { return err diff --git a/cli/command/stack/remove.go b/cli/command/stack/remove.go index 8324ba09a2..e4dd913dbd 100644 --- a/cli/command/stack/remove.go +++ b/cli/command/stack/remove.go @@ -19,7 +19,7 @@ func newRemoveCommand(dockerCli command.Cli) *cobra.Command { Args: cli.RequiresMinArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts.Namespaces = args - if dockerCli.ClientInfo().HasKubernetes { + if dockerCli.ClientInfo().HasKubernetes() { kli, err := kubernetes.WrapCli(dockerCli, cmd) if err != nil { return err diff --git a/cli/command/stack/services.go b/cli/command/stack/services.go index b3331c3df4..7f427c170e 100644 --- a/cli/command/stack/services.go +++ b/cli/command/stack/services.go @@ -19,7 +19,7 @@ func newServicesCommand(dockerCli command.Cli) *cobra.Command { Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts.Namespace = args[0] - if dockerCli.ClientInfo().HasKubernetes { + if dockerCli.ClientInfo().HasKubernetes() { kli, err := kubernetes.WrapCli(dockerCli, cmd) if err != nil { return err diff --git a/cli/flags/common.go b/cli/flags/common.go index a8e2840426..e2875563f0 100644 --- a/cli/flags/common.go +++ b/cli/flags/common.go @@ -54,7 +54,8 @@ func (commonOpts *CommonOptions) InstallFlags(flags *pflag.FlagSet) { 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.StringVar(&commonOpts.Orchestrator, "orchestrator", "", "Which orchestrator to use with the docker cli (swarm|kubernetes)") + flags.StringVar(&commonOpts.Orchestrator, "orchestrator", "", "Which orchestrator to use with the docker cli (swarm|kubernetes) (default swarm) (experimental)") + flags.SetAnnotation("orchestrator", "experimentalCLI", nil) // TODO use flag flags.String("identity"}, "i", "", "Path to libtrust key file") diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 20bcf393fa..3085990cc8 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -220,7 +220,7 @@ func hideUnsupportedFeatures(cmd *cobra.Command, details versionDetails) { osType := details.ServerInfo().OSType hasExperimental := details.ServerInfo().HasExperimental hasExperimentalCLI := details.ClientInfo().HasExperimental - hasKubernetes := details.ClientInfo().HasKubernetes + hasKubernetes := details.ClientInfo().HasKubernetes() cmd.Flags().VisitAll(func(f *pflag.Flag) { hideFeatureFlag(f, hasExperimental, "experimental") @@ -261,7 +261,7 @@ func areFlagsSupported(cmd *cobra.Command, details versionDetails) error { clientVersion := details.Client().ClientVersion() osType := details.ServerInfo().OSType hasExperimental := details.ServerInfo().HasExperimental - hasKubernetes := details.ClientInfo().HasKubernetes + hasKubernetes := details.ClientInfo().HasKubernetes() hasExperimentalCLI := details.ClientInfo().HasExperimental errs := []string{} @@ -301,7 +301,7 @@ func areSubcommandsSupported(cmd *cobra.Command, details versionDetails) error { clientVersion := details.Client().ClientVersion() hasExperimental := details.ServerInfo().HasExperimental hasExperimentalCLI := details.ClientInfo().HasExperimental - hasKubernetes := details.ClientInfo().HasKubernetes + hasKubernetes := details.ClientInfo().HasKubernetes() // Check recursively so that, e.g., `docker stack ls` returns the same output as `docker stack` for curr := cmd; curr != nil; curr = curr.Parent() { diff --git a/codecov.yml b/codecov.yml index ec7d4e2127..f589c2ac71 100644 --- a/codecov.yml +++ b/codecov.yml @@ -17,3 +17,6 @@ ignore: - "**/internal/test" - "vendor/*" - "cli/compose/schema/bindata.go" + - "cli/command/stack/kubernetes/api/openapi" + - "cli/command/stack/kubernetes/api/client" + - ".*generated.*" \ No newline at end of file diff --git a/docs/yaml/yaml.go b/docs/yaml/yaml.go index a92688892d..d909e45336 100644 --- a/docs/yaml/yaml.go +++ b/docs/yaml/yaml.go @@ -15,14 +15,17 @@ import ( ) type cmdOption struct { - Option string - Shorthand string `yaml:",omitempty"` - ValueType string `yaml:"value_type,omitempty"` - DefaultValue string `yaml:"default_value,omitempty"` - Description string `yaml:",omitempty"` - Deprecated bool - MinAPIVersion string `yaml:"min_api_version,omitempty"` - Experimental bool + Option string + Shorthand string `yaml:",omitempty"` + ValueType string `yaml:"value_type,omitempty"` + DefaultValue string `yaml:"default_value,omitempty"` + Description string `yaml:",omitempty"` + Deprecated bool + MinAPIVersion string `yaml:"min_api_version,omitempty"` + Experimental bool + ExperimentalCLI bool + Kubernetes bool + Swarm bool } type cmdDoc struct { @@ -43,6 +46,9 @@ type cmdDoc struct { Deprecated bool MinAPIVersion string `yaml:"min_api_version,omitempty"` Experimental bool + ExperimentalCLI bool + Kubernetes bool + Swarm bool } // GenYamlTree creates yaml structured ref files @@ -110,6 +116,15 @@ func GenYamlCustom(cmd *cobra.Command, w io.Writer) error { if _, ok := curr.Annotations["experimental"]; ok && !cliDoc.Experimental { cliDoc.Experimental = true } + if _, ok := curr.Annotations["experimentalCLI"]; ok && !cliDoc.ExperimentalCLI { + cliDoc.ExperimentalCLI = true + } + if _, ok := curr.Annotations["kubernetes"]; ok && !cliDoc.Kubernetes { + cliDoc.Kubernetes = true + } + if _, ok := curr.Annotations["swarm"]; ok && !cliDoc.Swarm { + cliDoc.Kubernetes = true + } } flags := cmd.NonInheritedFlags() @@ -186,6 +201,15 @@ func genFlagResult(flags *pflag.FlagSet) []cmdOption { if v, ok := flag.Annotations["version"]; ok { opt.MinAPIVersion = v[0] } + if _, ok := flag.Annotations["experimentalCLI"]; ok { + opt.ExperimentalCLI = true + } + if _, ok := flag.Annotations["kubernetes"]; ok { + opt.Kubernetes = true + } + if _, ok := flag.Annotations["swarm"]; ok { + opt.Kubernetes = true + } result = append(result, opt) }) diff --git a/e2e/compose-env.yaml b/e2e/compose-env.yaml index 0e0c3062dc..d77427a253 100644 --- a/e2e/compose-env.yaml +++ b/e2e/compose-env.yaml @@ -8,8 +8,6 @@ services: image: 'docker:${TEST_ENGINE_VERSION:-edge-dind}' privileged: true command: ['--insecure-registry=registry:5000'] - depends_on: - - registry notary-server: image: 'notary:server-0.4.2' diff --git a/e2e/stack/remove_test.go b/e2e/stack/remove_test.go index e3f14061e5..d5f1858c76 100644 --- a/e2e/stack/remove_test.go +++ b/e2e/stack/remove_test.go @@ -2,7 +2,6 @@ package stack import ( "fmt" - "os" "strings" "testing" @@ -21,9 +20,7 @@ func TestRemove(t *testing.T) { deployFullStack(t, stackname) defer cleanupFullStack(t, stackname) - result := icmd.RunCmd(shell(t, "docker version")) - fmt.Println(result.Stdout(), os.Getenv("DOCKER_HOST"), os.Getenv("TEST_DOCKER_HOST")) - result = icmd.RunCmd(shell(t, "docker stack rm %s", stackname)) + result := icmd.RunCmd(shell(t, "docker stack rm %s", stackname)) result.Assert(t, icmd.Expected{Err: icmd.None}) golden.Assert(t, result.Stdout(), "stack-remove-success.golden") diff --git a/kubernetes/README.md b/kubernetes/README.md index 96f78d9328..7d5e1fd0b2 100644 --- a/kubernetes/README.md +++ b/kubernetes/README.md @@ -1,5 +1,4 @@ # Kubernetes client libraries This package (and sub-packages) holds the client libraries for the kubernetes integration in -the docker platform. Most of the code is currently generated but will evolved quickly in -the next months. \ No newline at end of file +the docker platform. Most of the code is currently generated. \ No newline at end of file