meshery-istio icon indicating copy to clipboard operation
meshery-istio copied to clipboard

[Patterns] Notification of Dynamic Registration Failure

Open leecalcote opened this issue 4 years ago • 6 comments

Enhancement Request: Add info log to verify registration of pattern components (or failure to register pattern components).

Context: Going forward PatternOps (pattern-based operations) will be used for all of the adapter operations. If an adapter fails to register is capabilities (register its operations), we should denote this in the UI eventually, so the user can see this in the Notification Center.

Immediately, providing an INFO log will be an improvement.

Originally posted by @leecalcote in https://github.com/meshery/meshery-istio/pull/269#discussion_r704017674

leecalcote avatar Sep 09 '21 20:09 leecalcote

@leecalcote the issue with sending any notification on failing registration is that "events" bidirectional connection is opened by Meshery Server (which acts like a client to meshery adapter) when it initiates a request on the adapter while the registration process is initiated by the meshery adapter (in this case adapter is the client). Implementing this would be a bit more trickier than sending a WARN event to meshery because there is no channel to send the message.

Here's what we can do though: Right now, every operation that Meshery Server performs on Adapters is bidirectional but that does not guarantee that channels associated with operations will actually be used for streaming the events infact In case of concurrent operations, channels gets replaced (due to its global nature). This is not ideal but this is what happens. We can create a new gRPC bidirectional RPC only for the purpose of streaming events. In this case, meshery server will execute this operation during bootup and adapter will store a global events channel which will be used to transmit all sort of events, basically building on the principle "decouple production of events from consumption of events".

Having said that, the my above suggestion seems bad. Can't think of another better solution to the problem at the moment.

// @Revolyssup

tangledbytes avatar Sep 09 '21 21:09 tangledbytes

For now, let's change this to minimally providing an INFO log in stdout. Example of successfully registered Istio adapter:

➜  meshery-istio git:(master) make run
DEBUG=true GOPROXY=direct GOSUMDB=off go run main.go
INFO[2021-09-29T17:06:48-05:00] Adaptor Listening at port: 10000              app=istio-adaptor
INFO[2021-09-29T17:07:46-05:00] Creating instance                             app=istio-adaptor
INFO[2021-09-29T17:07:46-05:00] Listing Operations                            app=istio-adaptor
INFO[2021-09-29T17:07:47-05:00] Creating instance                             app=istio-adaptor
INFO[2021-09-29T17:07:49-05:00] Creating instance                             app=istio-adaptor

No indication of success or failure.

leecalcote avatar Sep 29 '21 22:09 leecalcote

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Nov 04 '21 00:11 stale[bot]

Something to be addressed in v0.7.0

leecalcote avatar Nov 04 '21 03:11 leecalcote

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Dec 09 '21 04:12 stale[bot]

@leecalcote Given the new EventStreamer, we can do this now. The only caviat is since we are not currently persisting or maintaining history, if the client connects after the registration has been done then the client won't be notified. Although if the client is already connected, they would get the notification of components being registered. Should I add that feature and close this issue?

Revolyssup avatar Sep 25 '22 22:09 Revolyssup