fail soft when crds are not installed

[LiorLieberman]

What type of PR is this?
/kind bug

What this PR does / why we need it:
Execution should work regardless of whether CRDs are installed or not
Which issue(s) this PR fixes:

Fixes #138

Does this PR introduce a user-facing change?:

Fix errors when CRDs are not installed in the cluster

[mlavacca]

Can we move this check to the caller logic and return a typed error from the provider, instead of having the same logic in each provider? Something like what I suggested here

[dpasiukevich]

/ltgm
/hold to get every review

[LiorLieberman]

Can we move this check to the caller logic and return a typed error from the provider, instead of having the same logic in each provider? Something like what I suggested here

I dont think we should wrap an already NotFound error in another custom NotFound type.

Also, I less like the idea of a list of providers that participated in the translation. I think providers can still participate in the translation with ingress annotations and without CRDs installed.

It is not ideal to have the same logic - hence a helper might be the way but I think we can not take it out to the caller, because we need to continue the execution of the provider even if the CRDs are not installed.

cc: @levikobi

[levikobi]

This looks good to me also TBH

I would have preferred having the NotFound check once at the caller level, but the caller doesn't directly call readTCP... or readUDP... - it calls a method that aggregates the reading of specific CRDs. Since this mid level function that reads everything needs to keep going even if TCP for example isn't installed, we would have to make the check there...

The main issue is that we might have one error of resources not found, and another error since there was an actual problem reading from the cluster, and one error might override the other

In other words, even though I would have preferred it more, I don't see how we can move the check to the caller and preserve the required functionality

[LiorLieberman]

/cc @mlavacca can you relate to the feedback and lgtm if this looks good to you ?

[mlavacca]

Oh yes, I get your point. It is not possible to move the logic entirely out of the provider, as the ReadResourcesFromCluster is provider-specific as well, and it calls the Resource-specific methods. I'm good with this implementation then 👍

/lgtm

[mlavacca]

/approve
/unhold

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LiorLieberman, mlavacca

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [LiorLieberman,mlavacca]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment