Fixing false warnings for the KudoOperatorTask

[zen-dog]

Summary:
in many cases, the task's OperatorVersion is being set by the dependency resolver during the package installation, so that it is valid for the field to be empty during verification.

Signed-off-by: Aleksey Dukhovniy alex.dukhovniy@googlemail.com

[nfnt]

LGTM, but I don't quite understand the summary. It's saying that this is being set by the dependency resolver so it's valid to be empty during verification, but we are adding an error here when it's empty? Or am I missing something? What does this have to do with false warnings if this is setting an error?

[ANeumann82]

lgtm

[ANeumann82]

LGTM, but I don't quite understand the summary. It's saying that this is being set by the dependency resolver so it's valid to be empty during verification, but we are adding an error here when it's empty? Or am I missing something? What does this have to do with false warnings if this is setting an error?

Yeah, took me a minute to understand it as well - Basically, the PR removes one of the checks that is one in Task.build, which is ok, because when the Operator is installed it does the lookup on installation.

So the in_cluster_resolver on the client (CLI) is ok with an unset OV, as it searches through the repository to find one.
The in_cluster_resolver on the server (controller) is not ok with an unset OV, as it only tries to fetch an installed one.

I think I would prefer to have the in_cluster_resolver for the server to be adjusted, but this is ok for now...

[zen-dog]

I think I would prefer to have the in_cluster_resolver for the server to be adjusted, but this is ok for now...

The idea behind the server in_cluster_resolver is to only check the existing dependencies in case e.g. somebody created the OV/I manually and forgot the dependencies. So it is basically a sanity check:

• all the resources should be installed
• it doesn't really resolve as there should be no ambiguity about which OV to take. An actual resolver would then have to download and install the packages and that's something we don't want to do on the server.

maybe we should rename it to smth. like in_cluster_dependency_checker to avoid this confusion. wdyt @ANeumann82 ?