Skip to content

Conversation

@aritrbas
Copy link
Collaborator

@aritrbas aritrbas commented Sep 16, 2025

This refactors the RouteWatcher component by separating the route watching functionality from the route handling logic.

Previously, the RouteWatcher used to (i) monitor netlink route updates and address changes and (ii) respond to network and IPAM configuration events via pubsub. Because of this, the RouteWatcher needed to be passed to the NetWatcher just so it could subscribe to events, and the event handling logic was mixed with route watching logic.

Now, we have a new handler component RouteHandler that implements three event handling methods:

  • OnNetDeleted(netDef *common.NetworkDefinition) error
  • OnNetAddedOrUpdated(netDef *common.NetworkDefinition) error
  • OnIpamConfChanged(oldPool, newPool *proto.IPAMPool) error

and holds a reference to RouteWatcher to perform route operations.

The RouteWatcher is now purely focused on route watching. It no longer subscribes to pubsub or performs any event handling. It is only responsible for netlink route/address monitoring.

NetWatcher and IPAM configuration changes now directly call the RouteHandler instead of sending pubsub events resulting in network and IPAM events being synchronously processed when they occur.

NetWatcher no longer needs to know about RouteWatcher, it only knows about the RouteHandler interface.

Events are now processed directly: NetWatcherRouteHandlerRouteWatcher
Previously: NetWatcherSendEventPubSubRouteWatcher.eventChanRouteWatcher

Made the changes on top of the nsk-split-svc branch for now, will rebase on top of master once those commits for single thread agent are merged.

@aritrbas aritrbas self-assigned this Sep 16, 2025
@aritrbas aritrbas force-pushed the abasu-route-watcher-rem-pubsub branch from 9763990 to eeacc54 Compare October 1, 2025 23:54
Copy link
Collaborator

@sknat sknat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks ! A few comments inline,
I think we could split the uplink_route_watcher in half and simplify the logic even more,
tell me if things are unclear !

Comment on lines 151 to 152
netWatcher.RegisterRouteWatcher(routeWatcher)
felixServer.RegisterRouteWatcher(routeWatcher)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to split the routeWatcher into two pieces:

  • the watching part (i.e. the WatchRoutes()) function
  • a routeHandler component (mostly OnNetDeleted ; OnNetAddedOrUpdated and OnIpamConfChanged)
    The latter could be instantiated in the felixServer, and called synchronously whenever events are received. This would remove the need for us pass the routeWatcher to the netWatcher of the felixServer

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should allow us to remove the NetDeleted and NetAddedOrUpdated events,
and replace them with eventChan <- NetAddedOrUpdated{} and eventChan <- NetDeleted{}

@aritrbas aritrbas force-pushed the abasu-route-watcher-rem-pubsub branch from eeacc54 to 4574268 Compare October 22, 2025 22:17
@aritrbas aritrbas changed the title remove route_watcher dependency on pubsub refactor: split routeWatcher into watcher and handler components Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants