spf13 / cobra

A Commander for modern Go CLI interactions
https://cobra.dev
Apache License 2.0
38.41k stars 2.86k forks source link

group flags for a subcommand #1327

Open medyagh opened 3 years ago

medyagh commented 3 years ago

in minikube our start command has many flags and we would like to group them for better readablility

$ minikube start --help
Starts a local Kubernetes cluster

Options:
      --addons=[]: Enable addons. see `minikube addons list` for a list of valid addon names.
      --apiserver-ips=[]: A set of apiserver IP Addresses which are used in the generated certificate for kubernetes.
This can be used if you want to make the apiserver available from outside the machine
      --apiserver-name='minikubeCA': The authoritative apiserver hostname for apiserver certificates and connectivity.
This can be used if you want to make the apiserver available from outside the machine
      --apiserver-names=[]: A set of apiserver names which are used in the generated certificate for kubernetes.  This
can be used if you want to make the apiserver available from outside the machine
      --apiserver-port=8443: The apiserver listening port
      --auto-update-drivers=true: If set, automatically updates drivers to the latest version. Defaults to true.

--base-image='gcr.io/k8s-minikube/kicbase:v0.0.17@sha256:1cd2e039ec9d418e6380b2fa0280503a72e5b282adea674ee67882f59f4f546e':
The base image to use for docker/podman drivers. Intended for local development.
      --cache-images=true: If true, cache docker images for the current bootstrapper and load them into the machine.
Always false with --driver=none.
      --cni='': CNI plug-in to use. Valid options: auto, bridge, calico, cilium, flannel, kindnet, or path to a CNI
manifest (default: auto)
      --container-runtime='docker': The container runtime to be used (docker, cri-o, containerd).
      --cpus=2: Number of CPUs allocated to Kubernetes.
      --cri-socket='': The cri socket path to be used.
      --delete-on-failure=false: If set, delete the current cluster if start fails and try again. Defaults to false.
      --disable-driver-mounts=false: Disables the filesystem mounts provided by the hypervisors
      --disk-size='20000mb': Disk size allocated to the minikube VM (format: [], where unit = b, k, m or
g).
      --dns-domain='cluster.local': The cluster dns domain name used in the Kubernetes cluster
      --dns-proxy=false: Enable proxy for NAT DNS requests (virtualbox driver only)
      --docker-env=[]: Environment variables to pass to the Docker daemon. (format: key=value)
      --docker-opt=[]: Specify arbitrary flags to pass to the Docker daemon. (format: key=value)
      --download-only=false: If true, only download and cache files for later use - don't install or start anything.
      --driver='': Driver is one of: virtualbox, parallels, vmwarefusion, hyperkit, vmware, docker, ssh (defaults to
auto-detect)
      --dry-run=false: dry-run mode. Validates configuration, but does not mutate system state
      --embed-certs=false: if true, will embed the certs in kubeconfig.
      --enable-default-cni=false: DEPRECATED: Replaced by --cni=bridge
      --extra-config=: A set of key=value pairs that describe configuration that may be passed to different components.
        The key should be '.' separated, and the first part before the dot is the component to apply the configuration to.
        Valid components are: kubelet, kubeadm, apiserver, controller-manager, etcd, proxy, scheduler
        Valid kubeadm parameters: ignore-preflight-errors, dry-run, kubeconfig, kubeconfig-dir, node-name, cri-socket,
experimental-upload-certs, certificate-key, rootfs, skip-phases, pod-network-cidr
      --feature-gates='': A set of key=value pairs that describe feature gates for alpha/experimental features.
      --force=false: Force minikube to perform possibly dangerous operations
      --force-systemd=false: If set, force the container runtime to use sytemd as cgroup manager. Defaults to false.
      --host-dns-resolver=true: Enable host resolver for NAT DNS requests (virtualbox driver only)
      --host-only-cidr='192.168.99.1/24': The CIDR to be used for the minikube VM (virtualbox driver only)
      --host-only-nic-type='virtio': NIC Type used for host only network. One of Am79C970A, Am79C973, 82540EM, 82543GC,
82545EM, or virtio (virtualbox driver only)
      --hyperkit-vpnkit-sock='': Location of the VPNKit socket used for networking. If empty, disables Hyperkit
VPNKitSock, if 'auto' uses Docker for Mac VPNKit connection, otherwise uses the specified VSock (hyperkit driver only)
      --hyperkit-vsock-ports=[]: List of guest VSock ports that should be exposed as sockets on the host (hyperkit
driver only)
      --hyperv-external-adapter='': External Adapter on which external switch will be created if no external switch is
found. (hyperv driver only)
      --hyperv-use-external-switch=false: Whether to use external switch over Default Switch if virtual switch not
explicitly specified. (hyperv driver only)
      --hyperv-virtual-switch='': The hyperv virtual switch name. Defaults to first found. (hyperv driver only)
      --image-mirror-country='': Country code of the image mirror to be used. Leave empty to use the global one. For
Chinese mainland users, set it to cn.
      --image-repository='': Alternative image repository to pull docker images from. This can be used when you have
limited access to gcr.io. Set it to "auto" to let minikube decide one for you. For Chinese mainland users, you may use
local gcr.io mirrors such as registry.cn-hangzhou.aliyuncs.com/google_containers
      --insecure-registry=[]: Insecure Docker registries to pass to the Docker daemon.  The default service CIDR range
will automatically be added.
      --install-addons=true: If set, install addons. Defaults to true.
      --interactive=true: Allow user prompts for more information

--iso-url=[https://storage.googleapis.com/minikube/iso/minikube-v1.17.0.iso,https://github.com/kubernetes/minikube/releases/download/v1.17.0/minikube-v1.17.0.iso,https://kubernetes.oss-cn-hangzhou.aliyuncs.com/minikube/iso/minikube-v1.17.0.iso]:
Locations to fetch the minikube ISO from.
      --keep-context=false: This will keep the existing kubectl context and will create a minikube context.
      --kubernetes-version='': The Kubernetes version that the minikube VM will use (ex: v1.2.3, 'stable' for v1.20.2,
'latest' for v1.20.3-rc.0). Defaults to 'stable'.
      --kvm-gpu=false: Enable experimental NVIDIA GPU support in minikube
      --kvm-hidden=false: Hide the hypervisor signature from the guest in minikube (kvm2 driver only)
      --kvm-network='default': The KVM network name. (kvm2 driver only)
      --kvm-qemu-uri='qemu:///system': The KVM QEMU connection URI. (kvm2 driver only)
      --memory='': Amount of RAM to allocate to Kubernetes (format: [], where unit = b, k, m or g).
      --mount=false: This will start the mount daemon and automatically mount files into minikube.
      --mount-string='/Users:/minikube-host': The argument to pass the minikube mount command on start.
      --namespace='default': The named space to activate after start
      --nat-nic-type='virtio': NIC Type used for nat network. One of Am79C970A, Am79C973, 82540EM, 82543GC, 82545EM, or
virtio (virtualbox driver only)
      --native-ssh=true: Use native Golang SSH client (default true). Set to 'false' to use the command line 'ssh'
command when accessing the docker machine. Useful for the machine drivers when they will not start with 'Waiting for
SSH'.
      --network='': network to run minikube with. Only available with the docker/podman drivers. If left empty, minikube
will create a new network.
      --network-plugin='': Kubelet network plug-in to use (default: auto)
      --nfs-share=[]: Local folders to share with Guest via NFS mounts (hyperkit driver only)
      --nfs-shares-root='/nfsshares': Where to root the NFS Shares, defaults to /nfsshares (hyperkit driver only)
      --no-vtx-check=false: Disable checking for the availability of hardware virtualization before the vm is started
(virtualbox driver only)
  -n, --nodes=1: The number of nodes to spin up. Defaults to 1.
  -o, --output='text': Format to print stdout in. Options include: [text,json]
      --ports=[]: List of ports that should be exposed (docker and podman driver only)
      --preload=true: If set, download tarball of preloaded images if available to improve start time. Defaults to true.
      --registry-mirror=[]: Registry mirrors to pass to the Docker daemon
      --service-cluster-ip-range='10.96.0.0/12': The CIDR to be used for service cluster IPs.
      --ssh-ip-address='': IP address (ssh driver only)
      --ssh-key='': SSH key (ssh driver only)
      --ssh-port=22: SSH port (ssh driver only)
      --ssh-user='root': SSH user (ssh driver only)
      --trace='': Send trace events. Options include: [gcp]
      --uuid='': Provide VM UUID to restore MAC address (hyperkit driver only)
      --vm=false: Filter to use only VM Drivers
      --vm-driver='': DEPRECATED, use `driver` instead.
      --wait=[apiserver,system_pods]: comma separated list of Kubernetes components to verify and wait for after
starting a cluster. defaults to "apiserver,system_pods", available options:
"apiserver,system_pods,default_sa,apps_running,node_ready,kubelet" . other acceptable values are 'all' or 'none', 'true'
and 'false'
      --wait-timeout=6m0s: max time to wait per Kubernetes or host to be healthy.

Usage:
  minikube start [flags] [options]

Use "minikube options" for a list of global command-line options (applies to all commands).

so something like

$ minikube start --help
Starts a local Kubernetes cluster

**Docker Driver Options:**
....
....

**Virtuablox Driver Options:**
...
...

** Another Group 
...
...

is that possible?

marckhouzam commented 3 years ago

Hi @medyagh, yes this can be done. For example kubectl does it while using Cobra (you can try with kubectl -h). I haven't done it myself but I believe they do it using cobra.Command.SetUsageFunc() and cobra.Command.SetHelpFunc().

kubectl even provides a package you can use: "k8s.io/kubectl/pkg/util/templates" and the code is here: https://github.com/kubernetes/kubernetes/tree/master/staging/src/k8s.io/kubectl/pkg/util/templates

Here is an example PR that grouped commands for helm based on that package: https://github.com/helm/helm/pull/7917/files Be aware that we reverted this PR in the end because the kubectl package had impacts on the help output that went beyond command grouping. But these impacts may be ok for you, or you may find a way to work around it. See this comment for the impacts that we found: https://github.com/helm/helm/pull/7917#issuecomment-629675187

medyagh commented 3 years ago

https://github.com/helm/helm/pull/7917/files

thanks for the response, I know about the grouping "the commands", (we have added that feature to minikube using the kubectl's lib), but that does Not help with adding groups for Flags. (it only helps with adding groups for Sub Commands) but I was hoping there was a native feature to group the flags of a command too.

for example i would like to see this to give me all of the flags but grouped into different groups

minikube start --help
Groups 1 flags
      . 
      . 
 Groups 2 flags
      . 
      . 
marckhouzam commented 3 years ago

Maybe you can give your support to #1003 which seems to implement this.

github-actions[bot] commented 3 years ago

This issue is being marked as stale due to a long period of inactivity

marians commented 3 years ago

+1 - this would be a great tool to improve the UX for more complex CLI commands. Curious to hear if anybody has found a way to solve this.

marians commented 2 years ago

@marckhouzam My understanding of #1003 is that it implements grouping of subcommands, not flags.

johnSchnake commented 2 years ago

I think this would also be a really useful command; I'm not sure about implementation but happy to review any PRs for it.

heyjcollins commented 2 years ago

The ability to group flags would be really beneficial. +1 bump!

marians commented 2 years ago

I just noticed that @knqyf263 has implemented this. So all it takes is a release now, or am I wrong? IMO this feature alone would justify a release.

marckhouzam commented 2 years ago

I just noticed that @knqyf263 has implemented this. So all it takes is a release now, or am I wrong? IMO this feature alone would justify a release.

@marians That implementation is in another project 😉 https://github.com/aquasecurity/trivy/pull/2488 Those GitHub references are not very clear.

knqyf263 commented 2 years ago

Yes, it is not this project. Sorry for confusing you. @marians

marians commented 2 years ago

Oh dear, I didn't notice that. Thanks! And greetings to our partner Aqua.

Have you considered bringing this change here?

knqyf263 commented 2 years ago

It seems like the discussion is suspended. Do the maintainers agree with this enhancement? If yes, I'd be happy to contribute upstream.

marckhouzam commented 2 years ago

It seems like the discussion is suspended. Do the maintainers agree with this enhancement? If yes, I'd be happy to contribute upstream.

I got the impression that the implementations out there are project-specific (overriding the help text and adding specific flag groups). If we can get a PR with a generic solution with a clean API, then it would be appropriate for Cobra.

RainbowMango commented 2 years ago

I really like the feature. Now I'm using NamedFlagSets to group the flags in the Karmada project, see here for example.

The flags looks like

$ karmada-controller-manager --help
...
Generic flags:

      --bind-address string                                                                                                                                                                                                      
                The IP address on which to listen for the --secure-port port. (default "0.0.0.0")
      ....

Logs flags:

      --add-dir-header                                                                                                                                                                                                           
                If true, adds the file directory to the header of the log messages
      ....
knqyf263 commented 2 years ago

I got the impression that the implementations out there are project-specific

Yes, it is a workaround as cobra doesn't support the feature.

NamedFlagSets looks good to me. @RainbowMango Are you going to bring it upstream? https://github.com/kubernetes/component-base/blob/b5a495af30a7bb04642ce82f4816b47e75f78dbe/cli/flag/sectioned.go#L33-L41

RainbowMango commented 2 years ago

The maintainer might have better solutions.

knqyf263 commented 2 years ago

I've opened a PR so that we can start a discussion. https://github.com/spf13/cobra/pull/1778

nirs commented 6 months ago

@medyagh can you share a full example of minikube command with flag groups?

An example from real command can help to answer:

hitzhangjie commented 2 months ago

Here's a demo, actually it's really easy:

func Test_spf13_cmdflags_groupby(t *testing.T) {
    rootCmd := cobra.Command{
        Use:   "root",
        Short: "root cmd description",
        Long:  "root cmd description",
        Run: func(cmd *cobra.Command, args []string) {
            fmt.Printf("command args: %v\n", strings.Join(args, " "))
        },
    }

    groups := map[*pflag.FlagSet]string{}
    // group1
    fs1 := pflag.NewFlagSet("features", pflag.ExitOnError)
    fs1.Bool("feature1", true, "enable feature1")
    fs1.Bool("feature2", true, "enable feature2")
    fs1.Bool("feature3", true, "enable feature3")
    rootCmd.Flags().AddFlagSet(fs1)
    groups[fs1] = "features"

    // group2
    fs2 := pflag.NewFlagSet("patches", pflag.ExitOnError)
    fs2.Bool("patch1", true, "apply patch1")
    fs2.Bool("patch2", true, "apply patch2")
    rootCmd.Flags().AddFlagSet(fs2)
    groups[fs2] = "patches"

    // cobra will:
    // - call helpfunc to print help message if defined, otherwise
    // - call usagefunc to print help message if defined, otherwise
    // - call the generated default usagefunc
    //
    // here we use SetUsageFunc is enough.
    rootCmd.SetUsageFunc(func(c *cobra.Command) error {
        for fs, name := range groups {
            usage := fs.FlagUsages()
            idx := strings.IndexFunc(usage, func(r rune) bool {
                return r != ' '
            })
            desc := strings.Repeat(" ", idx) + name + ":"
            help := desc + "\n" + usage
            fmt.Println(help)
        }
        return nil
    })

    // run
    os.Args = []string{"root", "--feature3=false", "--patch2=false", "--help", "helloworld"}

    err := rootCmd.Execute()
    require.Nil(t, err)
}