Skip to content
This repository has been archived by the owner on Jul 25, 2022. It is now read-only.

Garden CLI Improvements #3

Closed
62 tasks done
vlerenc opened this issue Jan 27, 2018 · 0 comments
Closed
62 tasks done

Garden CLI Improvements #3

vlerenc opened this issue Jan 27, 2018 · 0 comments

Comments

@vlerenc
Copy link
Contributor

vlerenc commented Jan 27, 2018

Thank you for providing gardenctl. Here what I found out using it and how we can make it even better:

  • In general think about making all output (but the help) structured and think about allowing for format options like -o json and -o yaml (like kubectl) and then we can again introduce a table/prose mode again, but we should have a technical output mode before that (e.g. yaml in the beginning, see examples below)

  • Targeting projects missing completely, but that's more important for the operator than seeds (which can be automatically handled by gardenctl as that's the scope our users work in

  • It would be convenient to support a form of gardenctl target where users don't have to explicitly name the kind (gardenctl should "guess" the kind project, seed, or shoot if unambiguously possible, otherwise it should show the conflict and ask the user to name the kind explicitly):

    • We could introduce garden clusters into the configuration, e.g. .garden/config:
      gardenClusters:
      - name: dev
        kubeConfig: ~/clusters/garden-dev/kubeconfig.yaml
      - name: prod
        kubeConfig: ~/clusters/garden-prod/kubeconfig.yaml
    • If nothing is in target, the argument may be a seed, project, or shoot (seed and project won't conflict in most cases, project and shoot could conflict sometimes), so gardenctl should take the argument and compare it against all seeds, all projects, and all shoots
    • If a project or seed is in target, the argument is always a shoot, so it's unambiguous anyway
    • Note 1: General expected behaviour in all cases:
      • If none matches, issue an error
      • If one matches, take it
      • If more than one matches, issue an error, output the matches, and ask the user for disambiguation
    • Note 2: As an extension of the above, plan to add the following as next step:
      • Allow for a * wildcard, e.g. foo* would match with foot or foo-bar, but not with my-foot (*foo* would match that as well)
      • Employ algorithms like https://en.wikipedia.org/wiki/Levenshtein_distance to measure the differences and pick the one with the shortest difference, but only up to a certain (ideally configurable in some file like e.g. .garden/config) limit
  • Introduce a command like gardenctl get [(garden|project|seed|shoot) <name>] that shows the seed, project, or shoot resource (in yaml, similar to kubectl ... -o yaml) as that's often already sufficient for an operator (if argument is omitted, show currently targeted resource):

    • Garden: Garden KUBECONFIG
    • Seed: Seed secret (soon actual seed CRD with kubernetes/garden-operator#259)
    • Project: Project namespace
    • Shoot: Shoot CRD
  • Consider renaming gardenctl get (gardens|projects|seeds|shoots|issues) into gardenctl ls (projects|seeds|shoots|issues), because that would feel more natural and wouldn't clash with get (ok, kubectl also uses get for both use cases, but that always feels wrong and commands such as aws or docker and the like all have an ls command)

  • When a garden, seed or shoot is targeted, show the location of the cached kubeconfig (for seeds and shoots), so that the user can immediately export it (e.g. even automatically within a bash alias/function, i.e. in a structured way), if he intends to work with that cluster from now on (instead of gardenctl kubeconfig -- ... which is more verbose and lacks command line completion)

  • Do not duplicate kubeconfigs in .garden/cache/tmp, but instead maintain the target as reference in e.g. in a file like .garden/target that contains the following (which would make it possible for users to embed that in their PS1 variable/command prompt):

    • If only the garden cluster is in target, the file contains:

      target:
      - kind: garden
        name: dev
    • If a project is in target, the file contains:

      target:
      - kind: garden
        name: dev
      - kind: project
        name: foo-bar
    • If a seed is in target, the file contains:

      target:
      - kind: garden
        name: dev
      - kind: seed
        name: seed-aws-eu1
    • If a shoot is in target and the user reached it via a project, the file contains:

      target:
      - kind: garden
        name: dev
      - kind: project
        name: foo-bar
      - kind: shoot
        name: cl-54321
    • If a shoot is in target and the user reached it via a seed, the file contains:

      target:
      - kind: garden
        name: dev
      - kind: seed
        name: seed-aws-eu1
      - kind: shoot
        name: cl-54321
    • If the user runs gardenctl drop without a kind, the last entry from the target "stack" is "popped" until the target "stack" is empty (in which case gardenctl should issue an error)

    • If the user runs gardenctl drop project or gardenctl drop seed while targeting a shoot, both project/seed and the shoot are "popped" from the target "stack"

    • Instead, gardenctl drop is expecting some "target" right now (the concrete resource maybe, but why?) and the sub command help (like for many other sub commands) is completely missing and instead I get some "Cobra" hint:

      > g drop
      Command must be in the format: drop [target]
      
      > g drop shoot
      Command must be in the format: drop [target]
      
      > g drop --help
      A longer description that spans multiple lines and likely contains examples
      and usage of using your command. For example:
      
      Cobra is a CLI library for Go that empowers applications.
      This application is a tool to generate the needed files
      to quickly create a Cobra application.
      
      Usage:
        gardenctl drop [flags]
      
      Global Flags:
            --cache int       activate 1 / deactivate 0 caching (default 1)
            --config string   config file (default is $HOME/.gardenctl.yaml)

      Expected is the behaviour from above and proper sub command help for all sub commands.

  • Do not show the seed cluster namespace when listing shoots (e.g. shoot-garden-mitsubishi-clust4vora), but depending on the target:

    • If neither seed nor project is in target and someone runs gardenctl ls shoots (group rather by project than seed):
    projects:
    - project: foo-bar
      shoots:
      - cl-12345
    - project: john-doe
      shoots:
      - cl-54321
    - project: some-thing
      shoots:
      - cl-thing
    • If a project is in target and someone runs gardenctl ls shoots:
    seeds:
    - seed: seed-aws-eu1
      shoots:
      - cl-12345
      - cl-54321
    - seed: seed-aws-na1
      shoots:
      - cl-thing
    • If a seed is in target and someone runs gardenctl ls shoots:
    projects:
    - project: foo-bar
      shoots:
      - cl-12345
    - project: john-doe
      shoots:
      - cl-54321
  • Do not hide IaaS CLI stdout/err, e.g. when launching the command wrongly, I don't know what went wrong:

    > g aws s3 nonsense
    panic: Please make sure to use a valid aws command
    
    goroutine 1 [running]:
    [callstack...]

    Expected:

    > aws s3 nonsense
    usage: aws [options] <command> <subcommand> [<subcommand> ...] [parameters]
    To see help text, you can run:
    
      aws help
      aws <command> help
      aws <command> <subcommand> help
    aws: error: argument subcommand: Invalid choice, valid choices are:
    
    ls                                       | website
    cp                                       | mv
    rm                                       | sync
    mb                                       | rb
    presign
    
  • The same also happens with kubectl:

    > g kubectl get nodes
    panic: exit status 1
    
    goroutine 1 [running]:
    [callstack...]

    Expected was the error message of kubectl.

  • Generally, in case of expected (user) errors (like above or below), do not write a call stack (write a callstack only when something unanticipated happens, e.g. the program reaches its outermost catch block):

> $ g download tf shoot-garden-core-itpa-436 infra
panic: configmaps "itpa-436.infra.tf-config" not found <-- that is helpful

goroutine 1 [running]:                                 <-- that is not helpful
[callstack...]
  • Remove gardectl get all as that's implicit if the user hasn't targeted any project or seed before anyhow with gardectl get shoots
  • Remove gardenctl target direct as that's not part of the spec Marshalling out of struct #2 and confusing, because the word direct is used to target shoots, but the word has no relation to that kind
  • Instead, add the following optional options to garden target: --garden|-g, --project|-p or --seed|-s (never allow both, project and seed, at the same time)
  • Make gardenctl get issues more helpful (and faster if possible? why is it so slow?):
    • Do not show the namespace, but the project as this is the main hierarchical level and show it first
    • Show the shoot cluster resource status (without uid and gardenOperator and possibly later also other black-listed fields not helpful for this command)
    issues:
    - project: foo-bar
      seed: seed-aws-eu1
      shoot: cl-54321
      status:
        lastError: "Failed to create Shoot cluster (Errors occurred during parallel execution:
          '(CloudBotanist).DeployInfrastructure' returned 'Terraform execution job could
          not be completed. The following issues have been found in the logs:\n\n-> Pod
          'paj7wlu4tu.infra.tf-job-79984' reported:\n* aws_vpc.vpc: 1 error(s) occurred:\n*
          aws_vpc.vpc: Error creating VPC: VpcLimitExceeded: The maximum number of VPCs
          has been reached.\n\tstatus code: 400, request id: <omitted>')"
        lastOperation:
          description: "Failed to create Shoot cluster (Errors occurred during parallel
            execution: '(CloudBotanist).DeployInfrastructure' returned 'Terraform execution
            job could not be completed. The following issues have been found in the logs:\n\n->
            Pod 'paj7wlu4tu.infra.tf-job-79984' reported:\n* aws_vpc.vpc: 1 error(s) occurred:\n*
            aws_vpc.vpc: Error creating VPC: VpcLimitExceeded: The maximum number of VPCs
            has been reached.\n\tstatus code: 400, request id: <omitted>')"
          lastUpdateTime: 2017-12-05T10:01:15Z
          progress: 36
          state: Failed
          type: Create
  • g download tf NAME infra wasn't doing anything for me, it just ended
  • Remove the need to name the cluster in above command
  • g show vpn-seed isn't showing all control plane pods that contain a vpn (sidecar) container (e.g. Prometheus needs the vpn (sidecar) container as well)
  • g show (ui|dashboard) isn't showing the pod information on the command line like g show (prometheus|grafana|alertmanager) does (all show sub commands should do that)
  • g show ui was opening the (singular) landing page, instead of opening the corresponding gardener UI by looking into the garden cluster (and then e.g. the ingress gardener-ingress resource)
  • g (show|logs) tf (infra|dns|ingress) not yet implemented (watch out, a.) there may be many terraform pods, pick the latest/running and b.) terraform pods may alreadybe gone, when the operation completed)
  • g logs operator wasn't doing anything for me (when a shoot was in target), it just ended
  • g logs operator should show the operator logs filtered by the currently targeted shoot (I targeted the shoot cluster that appeared last in the full log)
  • g logs dashboard wasn't doing anything for me, it just ended
  • g logs addon-manager wasn't doing anything for me, it just ended
  • g logs (prometheus|grafana|alertmanager) failed with an exception right away
  • What is the help entry save [config] supposed to do/mean (not in spec Marshalling out of struct #2)?
  • Remove help entry (and logic behind that if implemented) for --config string config file (default is $HOME/.gardenctl.yaml) (not in spec Marshalling out of struct #2) and use a default such as $HOME/.garden/config or allow for an environment variable such as GARDENCONFIG that points to said file; mention that in the help
  • Remove help entry (and logic behind that if implemented) for --cache int activate 1 / deactivate 0 caching (default 1) (not in spec Marshalling out of struct #2) and always cache unless user runs gardenctl with the --no-cache option; mention said option --no-cache in the help
  • Introduce the following short forms for gardenctl kubectl (can be, but doesn't necessarily have to be mentioned in the help if it would make it unreadable):
    • gardenctl k substitutes gardenctl kubectl
    • gardenctl ks substitutes gardenctl kubectl --namespace=kube-system
    • gardenctl ka substitutes gardenctl kubectl --all-namespaces=true
  • Remove the welcome line in the help, especially since it says Gardenctl, which is incorrect (not the name of the command, which is case-sensitive)
  • Rearrange the help as it uses alphabetical order which is hard to read and makes it difficult for the user to get the gist of what gardenctl is actually doing:
Usage:
  gardenctl <command>

Available Commands:
  ls          (gardens|projects|seeds|shoots|issues)   list all resource instances, e.g. list of shoots
  target      (garden|project|seed|shoot) <name>      set scope for next operations
  drop        [(garden|project|seed|shoot)]           drop scope for next operations (default: last target)
  get         [(garden|project|seed|shoot) <name>]    get single resource instance, e.g. CRD of a shoot (default: current target)

  download    tf (infra|dns|ingress)                  download terraform configuration/state for local execution for the targeted shoot
  show        (operator|ui|                           show details about endpoint/service and open in Chrome if applicable
               tf (infra|dns|ingress)|                (tf sub commands require targeted shoot)
               api|scheduler|controller-manager|etcd-operator|etcd-main|etcd-events|
               addon-manager|vpn-seed|vpn-shoot|auto-node-repair|
               dashboard|prometheus|grafana|alertmanager)
  logs        (operator|ui|                           show and optionally follow logs of given component
               tf (infra|dns|ingress)|                (tf sub commands require targeted shoot)
               api|scheduler|controller-manager|etcd-operator|etcd-main|etcd-events|
               addon-manager|vpn-seed|vpn-shoot|auto-node-repair|
               dashboard|prometheus|grafana|alertmanager)

  kubectl     <args>
  aws         <args>
  az          <args>
  gcloud      <args>
  openstack   <args>

Available Options:
      --no-cache        do not cache KUBECONFIG files
  -h, --help            help for gardenctl

Use "gardenctl <command> --help" for more information about a given specific command.
Configuration and KUBECONFIG file cache located $GARDENCTL_HOME or ~/.garden (default).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants