nad: Skip NADs with empty config

[oshoval]

What this PR does / why we need it:
Until now, a NAD with empty config, was considered invalid,
causing an error / rejecting it on the webhook.
Istio creates such NADs, so we can't consider them as fatal,
once we list all the NADs in a namespace.
Otherwise the webhook will block VM creation [1] in case it uses
such NAD.

Therefore just ignore those.

[1] virtualmachine-controller    Error creating pod: admission webhook "ipam-claims.k8s.cni.cncf.io" denied the request: failed to extract CNI configuration from NAD: unexpected end of JSON input

Example of resource that create such NAD:

apiVersion: maistra.io/v1
kind: ServiceMeshMemberRoll
metadata:
  name: default
  namespace: istio-system
spec:
  members:
  - non-udn-ns

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Release note:

None

[oshoval]

Hi @maiqueb
I can just change NewConfig but it will ignore empty NADs even on NADs that were supposed to belong to OVN-K
(unless for example we filter by istio label, but since this label might get changed and due to the relative complexity for just this purpose, i don't think this is good idea)
Would you prefer the current solution or just to change NewConfig ?
Once we converge i can add UT, thanks

[maiqueb]

Hi @maiqueb I can just change NewConfig but it will ignore empty NADs even on NADs that were supposed to belong to OVN-K (unless for example we filter by istio label, but since this label might get changed and due to the relative complexity for just this purpose, i don't think this is good idea) Would you prefer the current solution or just to change NewConfig ? Once we converge i can add UT, thanks

@oshoval how can an empty NAD belong to OVN-K ?

For a NAD to be associated w/ a plugin, it must feature the name of the plugin in their type. If it's empty, it can't belong to it. Hence, nothing to do.

[maiqueb]

Can you coalesce this check to a single place ?

[oshoval]

@oshoval how can an empty NAD belong to OVN-K ?

of course it shouldn't, i mean just because of a bug (which was the original reason we added it in the first place)
anyhow will consolidate

[oshoval]

Consolidated (an empty config is robust in the current callers, they will just "continue", each on its own way)
I don't think it deserve UT, and also there isn't UT for the current function atm

[maiqueb]

/lgtm

[maiqueb]

/lgtm

/approve

[kubevirt-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maiqueb

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 [maiqueb]

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

[oshoval]

I will just issue a release and we make sure it works with it, as it shouldn't be risky

[oshoval]

btw note please that sometimes doing double tag on the same hash cause mistakes on CNAO
(some of them were solved, some not sure yet)

• 05b47ce (tag: v0.2.0, tag: v0.1.11-alpha) Merge pull request config/manager, ipam-ext Deployment: Remove limits, Increase reqeusts #88 from RamLavi/remove_limit