dmsg hypervisor #563
dmsg hypervisor #563
Conversation
|
Closes #560 |
* Reserving route IDs and adding rules to visors is now split into two communication steps. * Improved readability and testability of the setup procedure but splitting responsibilities to additional structures; setup.idReservoir, setup.RulesMap * Improved logging for setup procedure. * Slightly tweaked setup.Protocol to accommodate aforementioned changes.
Fix visor node restart with routes.
This was removed for some reason, but it needs to exist in order to reestablish transports on visor restart.
…sports Re-added initTransports for transport.Manager
|
Btw, will be good if you can provide steps of how this can be tested. |
|
This can be tested with
From now on you can test the hypervisor API, for example you can |
|
Note however that by removing |
There was a problem hiding this comment.
Note however that by removing
noise.RPCClientDialersnow there is no retrial in theDialoperation betweenvisorandhypervisor. It could be added if deemed necessary in line https://github.com/skycoin/skywire/pull/563/files#diff-8b79394cd85c849e746408355107d3b6R241 using annetutil.Retrier.
Yes, we will need retry logic. Please implement.
Where is the endpoint documentation kept? Are we still updating this Postman file? |
I didn't update it, will do it as part of the requested changes. |
pkg/visor/visor.go
Outdated
| var conn net.Conn | ||
| var err error | ||
|
|
||
| err = retrier.Do(func() error { |
There was a problem hiding this comment.
The redial logic here is not the same as what we had previously in:
https://github.com/skycoin/dmsg/blob/70f4c32a994fe7fa62c36049e3ce46d0dc5c8c20/noise/net.go#L55-L77
Please fix.
There was a problem hiding this comment.
To elaborate, if something breaks during rpc communication, we want to redial.
There was a problem hiding this comment.
To elaborate further, if hypervisor restarts, visor should be able to reconnect automatically to the hypervisor.
|
@ivcosla Just gave it a test run and it is working very well. Please fix the last request, thank you. |
…into feature/dmsg-hypervisor
| for { | ||
| if err := d.establishConn(); err != nil { | ||
| // Only return if not network error. | ||
| if err != dmsg.ErrRequestRejected { |
There was a problem hiding this comment.
Similar to *noise.RPCClientDialer check for net.Error type assertion here I think it would be useful to group all of dmsg network errors under a custom error type. Or at least those that we consider meaningful for checking outside the package.
Working state, integration tests are working, but hypervisor tests need fix.