Refactor KubectlClient module with focus on logging

[rafal-lal]

Generally ready to review, but not yet ready to merge as changes made here needs to be reflected in main cnti-testsuite.

Changes:

• add logging scopes with Log.for()
• clean many unneeded log messages (used during development?)
• refactor existing logs to be more informative, log.info should be clear message in english, log.debug should hold informations useful to debugging and not produce wall of logs with values of variables or other development leftovers. Also add log.trace usage for very verbose logs like outputs of commands etc.
• add throwing of exceptions in case of errors in CMD execution - one of the main problems with this shard in my opinion - lack of error handling, there was simply no way to tell if some of the methods failed or not. In case of the simpler ones Process::Status, stdout and stderr were returned in format of NamedTuple - so that was ok (some of the methods returned NamedTuple, some just Bool - this was standarised as well). It is not idomatic to crystal-lang Exceptions based approach but acceptable. In case of more complex methods, CLI command that fetches some resource from the kubernetes could fail, and method would ignore this fact and continue with the logic and return some value - in most cases empty one. This leaves the caller of the method in complete unknown - did the method fail?, did it finish succesfully and returned empty value?, is the empty value valid response or not?. Error handling is actaully very important in testsuite - lets look at scenario in which test case that checks if number of replicas can be changed is executed. Lets say some 3rd or 4th call to KubectlClient shard fails due to random connectivity error to the cluster. Testcase fails. Is the user notified that testcase failed due to network error and should be restarted or it fails without proper feedback leaving users with 0 points for it? Exceptions should be caught somewhere at testcase level and clearly described to users. In situation where exception is somewhat expected, it can be rescued and some adequate to sitation action can be taken. Reference Crystal's way to do error handling is by raising and rescuing exceptions.
• remove many non-generic Get submodule methods with universal Get::resource one - there were couple of methods
  repeating the very same pattern: kubectl get <kind> <resource_name>, but the kind was hardcoded to deployment, service, pods, nodes... The only changing value was kind. This is making this shard harder to maintain, debug and keep in consistent state, I've seen many duplicated functionality across this shard mostly due to lack of universality in methods.
• remove some of the most exotic methods e.g. container_digests_by_nodes and others - the reason was mainly that these methods were not used at all in main cnti-testsuite. The second reason was that these methods are just too specific to make sense. We can take golang kubectl-client package as reference. It mostly provides functions to fetch or post the resources, any aggregations or filtering is mainly specific to certain use cases and is done inside solutions code, not inside library. Some reasonable helpers are definitely okay, but here we have situation when we have those quite specific helpers, and they are just not used anywhere. Instead main cnti-testsuite uses this shard to fetch the resources (as it should be) and leaves tons of these helpers here, forgotten, unmainted, unused, filled with TODOs and develop time logs.
• remove commented out code
• add new file with Wait submodule with methods extracted from wider Get submodule - there were enough of them that creation of new submodule made sense
• add new constants
• simplify logic of some methods where possible
• add arguments and return types to many of the methods
• update test cases

This is not everything but enough as first step. Second would be to move this shard to main cnti-testsuite as maintaining those shards outside of main codebase is problematic in itself.

[svteb]

Minor inconsistencies:

• Sometimes when running a function from the same module you prefix it with KubectlClient::Module_name.function and at other times you only use function.
• Occasionally there is no info message printed when running a function, this is kind of inconsistent. Especially in Get the very first message is usually debug while in other modules the first message is info.
• Sometimes you print the result of function with info and other times not. Look at pods_by_resource_labels for example.
• I don't quite understand why you sometimes specify the amount of resources returned (this is only done with pods, I believe). I have no qualms about this but it feels a little exotic/purposeless.
• In the future we could consider making pod_ready and node_ready functions "unified", similarly to how it has been done with get_resource.

None of these propositions are too important so I'm approving.

[svteb]

I feel like using % is not exactly idiomatic to crystal, consider using #{}.

[rafal-lal]

Different use case, here Im filling previously defined template string and not create new string with variables.

[svteb]

This will probably result in some uneven formatting if cmd ever gets printed (kubectl get #{kind} --field-selector #{field_selector}), consider normalizing the spacing in some final command.

[rafal-lal]

Yes there will be one space more printed in log.trace. This is so NIT that I honestly don't want to bother with pushing new commit.

[svteb]

Defaulting all_namespaces to true here is a little destructive if a user passes namespace. Although I have no idea how to resolve it.

[rafal-lal]

That's true, Im hoping people will look up the signature before using. Hypothetically I could add validator, but that would mean adding validators to all methods for consistency - not something I would like to do.

[rafal-lal]

Minor inconsistencies:

* Sometimes when running a function from the same module you prefix it with `KubectlClient::Module_name.function` and at other times you only use `function`.

* Occasionally there is no `info` message printed when running a function, this is kind of inconsistent. Especially in `Get` the very first message is usually `debug` while in other modules the first message is `info`.

* Sometimes you print the result of function with `info` and other times not. Look at `pods_by_resource_labels` for example.

* I don't quite understand why you sometimes specify the amount of resources returned (this is only done with pods, I believe). I have no qualms about this but it feels a little exotic/purposeless.

* In the future we could consider making `pod_ready` and `node_ready` functions "unified", similarly to how it has been done with `get_resource`.

None of these propositions are too important so I'm approving.

• If I'm calling something from other KubectlClient submodule I need to use KubectlClient::Module_name.function, otherwise it wont be found, methods from the same submodule can be called with function.
• Its on purpose to not bloat the INFO logs. Reasoning is that methods which logs INFO are actually modifying something on the cluster, so it should be loudly reported. Those Gets use DEBUG as they only fetch data from the cluster, not that important unless investigating behavior closely.
• This is on purpose, look at pods_by_resource_labels end, it calls 2 other methods which are printing logs, printing here would mean duplication of logs.
• That is a little inconsistent. I might change it actually.
• Good point, but I would leave it for now.

[rich-l]

lgtm