fix(grpc): throw error on wrong gateway listener

[sskaur]

This PR adds an exception in cases of create/delete_listener where the gateway name is supplied along with traddr but the gateway name does not match the current gateway. Previously, no SPDK call would be made but the listener request would return successfully.

Thanks to @pgerv12 and @SeanP-2023 for finding this issue.

[trociny]

I believe it is expected behaviour, intended for multigateway configuration. Note, we skip creating the local spkd listener but we store it in the omap state, so other gateway (with this name) could detect it and create.

I think an improvement that still could be implemented would be to check if the name matches one of the group gateways and throw error if it does not. But currently we don't have such a list of gateways available to check against it.

[sskaur]

I believe it is expected behaviour, intended for multigateway configuration. Note, we skip creating the local spkd listener but we store it in the omap state, so other gateway (with this name) could detect it and create.

I think it's misleading for the grpc to return True when it's possible that no listener will get created. What happens when there's an error with the SPDK call on the other gateway?

[trociny]

I think it's misleading for the grpc to return True when it's possible that no listener will get created. What happens when there's an error with the SPDK call on the other gateway?

So, you propose just to disable a possibility to configure a listener for another gateway? Note, for a not listener case, when a spdk call should be executed on all gateways, if it succeeds on one spdk it does not guarantee it will succeed on all others. Also, after restart, when loading the stored state an spdk call may fail this time.

[caroav]

@trociny @sskaur do you all agree it is by design? If yes, then can we drop this PR?

[trociny]

AFAIR on one of the weekly meetings it was discussed that we would want this limitation (create/delete listener only with its "local" gateway) to avoid some corner cases. If I understood that correctly then this PR makes sense. @PepperJo can you confirm?

[caroav]

Yes, after re-reading this PR, I think that this limitation does makes sense. I think that in the future we might want to reconsider it, to allow more ease of use when configuring multi GWs (so user doesn't need to do it separately for each GW), but for now it might be good. @PepperJo agree?

[PepperJo]

@trociny @caroav Yes, I think this PR makes sense. For now I think this is the safest and easiest way to implement a "fix".
@trociny yes, in theory every request can fail but at least in the non-local case we always try to apply locally with SPDK before. That said even if we would test a request on all nodes before applying, it could still happen that at some point later when e.g. a new GW is added or a GW is restartet a config option cannot be applied and the GW fails. This we always have to handle.

[caroav]

@trociny based on latest comment from @PepperJo and based on the upstream discussion yesterday, I think we should merge this change.
@sskaur assuming no further comments from @trociny , will you be able to merge it please.