Validate KUDO installation before running commands #1113
Conversation
ANeumann82
requested review from
alenkacz,
gerred,
kensipe,
nfnt and
zen-dog
as code owners
# Conflicts: # pkg/kudoctl/util/kudo/kudo.go
There was a problem hiding this comment.
I have few nits on this PR but before I dive into that - what do you think about introducing a command instead? I think it's much nicer to have something like k kudo validate-install than --validate-install on every command of course we would have to do a good job documenting it. I like this model from linkerd they have a check command and run various checks on your cluster to help you figure out if linkerd is healthy.
Well, the origin of this PR is an issue that a user ran More important I think is future work, if we have more KUDO versions out in the wild. If operator1 upgrades the KUDO installation on the cluster, and operator2 tries to run a command on that cluster, it would be good for the operator to know that his KUDO control does not match the installed CRDs and manager. As already mentioned to @nfnt , the "validate-install" is more of an override to skip the validation in case you know what you're doing. From my perspective, the validation should be performed on every run. (It doesn't make sense for some subcommands like |
Start KUDO controller for IT
|
After thinking about it a bit more, i've removed the validation of the KUDO manager again. It's not required that the installed controller is matching versions, as long as the CRDs are compatible. The |
| @@ -31,7 +32,7 @@ type Client struct { | |||
| } | |||
|
|
|||
| // NewClient creates new KUDO Client | |||
| func NewClient(kubeConfigPath string, requestTimeout int64) (*Client, error) { | |||
| func NewClient(kubeConfigPath string, requestTimeout int64, validateInstall bool) (*Client, error) { | |||
There was a problem hiding this comment.
I wouldn't mind adding a test for this method actually verifying the behavior with validate=true 👼
There was a problem hiding this comment.
I've added a unit test, but I have a hard time coming up with a test in the current structure. I'll start a refactoring PR next, and going to tackle the whole testing thing there.
What this PR does / why we need it:
The PR validates that the KUDO CRDs are installed on the targeted cluster, and that the CRD version is the same as in the KUDO CLI. This is executed anytime a KudoClient is created.
It adds a global flag "--validate-installation" that defaults to "true" that allows a user to skip this validation and run the command anyway.
Fixes #1056