kinvolk/kube-spawn

Improving kube-spawn's code base

schu opened this issue · 2 comments

schu commented

An incomplete list of issues with kube-spawn'c current code base for documentation purposes.

  • goroutines are used for concurrency in several places (e.g. in DownloadK8sBins) but racy due to

    • insufficient error handling (multiple go routines write a single variable)
    • usage of log.Fatalf
    • multiple routines writing to stdout/err
  • A single cfg *config.ClusterConfiguration object is used

    • to store configuration parameters by the user
    • to store internal configuration
    • as a global context object throughout the whole code base

    With that, the code is very opaque and polluted with side effects.

    Instead, types, interfaces, methods and meaningful sub packages should be used where possible. As is, the code base is hard to maintain and improve, for example to add a gRPC daemon interface.

  • Type and function names are often confusing and not explanatory

  • Some parts of the code suffer from feature creep

  • Source code comments are often outdated / insufficient / confusing or non-existing where important

  • Currently, kube-spawn users have to set the numbers of nodes a cluster has during create unnecessarily.

I'm working on some of those in an attempt to untangle the code and hope to have a work in progress pull request soon.

schu commented

#265 is work in progress code to address points raised above.

AFAIK most of the issues above were already fixed via #265 a long time ago.
So I'm going to close it.
If anything is still left, let's create another issue.