kudobuilder / kudo

Kubernetes Universal Declarative Operator (KUDO)
https://kudo.dev
Apache License 2.0
1.18k stars 103 forks source link

`kudo update --wait` is racy #1465

Closed zen-dog closed 4 years ago

zen-dog commented 4 years ago

966 has introduced and #1461 refined waiting for a plan to finish on a particular instance. However, there is an underlying issue, where the instance state might be fetched before a particular update makes it to the IC (instance controller) and it will look like the plan is already done. Furthermore, without an active IAC, the update might trigger a new plan where an old plan is already running and so this code becomes even more brittle.

What happened:

What you expected to happen:

How to reproduce it (as minimally and precisely as possible):

Anything else we need to know?:

Environment:

kensipe commented 4 years ago

The work of #966 and #1461 was on install and did not include update although it was considered somewhat in the creation of the main wait function introduced in 1461.
It isn't clear what an IAC is... however currently kudo only allows for 1 active plan.

kensipe commented 4 years ago

Removing bug label... this isn't a bug... looking at kudo update there is no --wait flag... thus no bug.

from kudo master (which is v0.12.0++)

go run cmd/kubectl-kudo/main.go update --help
Update KUDO operator instance with new arguments. The update does not accept any arguments.

Usage:
  kubectl-kudo update [flags]

Examples:
  # Update dev-flink instance with setting parameter param with value value
  kubectl kudo update --instance dev-flink -p param=value

  # Update dev-flink instance in namespace services with setting parameter param with value value
  kubectl kudo update --instance dev-flink -n services -p param=value

Flags:
  -h, --help                         help for update
      --instance string              The instance name.
  -p, --parameter stringArray        The parameter name and value separated by '='
  -P, --parameter-file stringArray   YAML file with parameters

Global Flags:
      --home string           Location of your KUDO config. (default "/Users/kensipe/.kudo")
      --kubeconfig string     Path to your Kubernetes configuration file. (default "/Users/kensipe/.kube/config")
  -n, --namespace string      Target namespace for the object. (default "default")
      --request-timeout int   Request timeout value, in seconds.  Defaults to 0 (unlimited)
  -v, --v                     Log level for V logs
      --validate-install      Validate KUDO installation before running. (default true)
zen-dog commented 4 years ago

You're right, update --wait doesn't have this problem because there is no --wait flag for it. Let's hope that the developer implementing this feature will not reuse the WaitForInstance method as it will become racy. Case closed