Refactoring of "get instances" CLI command

[fabianbaier]

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/component kudoctl

What this PR does / why we need it:

This PR refactors the kubectl kudo get instances command. We are moving away from the dynamicClient for listing Instances and start using our own KUDO client similar to the install command.

Which issue(s) this PR fixes:

Fixes #548

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

[alenkacz]

I think I am a bit confused around some parts of the PR as a lot seems like a dead code to me 🤔 either I am not understanding the purpose of this or we copy-pasted a lot from install?

[alenkacz]

is this copy-paste error? How is skipInstance relevant to get instances? 🤔

[fabianbaier]

I wouldn't call it that. More good intensions to make parts of the CLI following same patterns/look alike.

[fabianbaier]

I think I am a bit confused around some parts of the PR as a lot seems like a dead code to me 🤔 either I am not understanding the purpose of this or we copy-pasted a lot from install?

The purpose of this is and was intended to bring the get command back in sync with the rest of the CLI. I see a lot of changes we made that haven't been made to the get part itself. E.g. we use no longer the dynamicClient, we follow a certain pattern in install (I use btw install as the gold standard for other components in the CLI and think we should use similar patterns to make the code look more homogenous).

There is also the lack of testing in get that I tried to kick off and start fixing.

Overall, I am more interested in creating patterns within our CLI, like using the Options struct within each command, having same patterns for parsing of parameters or validation of arguments before we go into more business logic that lives in private functions. Then standardizing in each component the error handling and log output, maybe with having -o yaml as option and so on. How do we get there, I don't know. I just would like to get there. Thought this PR would be the first step towards those goal.

[jbarrick-mesosphere]

I'm all for making this more testable 👍

[alenkacz]

Yeah, now it makes sense, when you removed the options that don't make sense for get instances and were just taken over from install.

Now one last copy-pasted line, if we remove that it's good to go :) YAY

[kensipe]

lgtm