Synchronization issue when context times out

[shreyb]

Last week, a few bad destination nodes made the token-push run time out. We saw the following (edited) log lines emitted:

2025-02-20 09:08:50	{"account":"sbndci","destPath":"/tmp/vt_****","level":"error","msg":"Could not copy source file to destination node","node":"NODE","rsyncOpts":"--perms --chmod=u=r,go=","sourcePath":"PATH","time":"2025-02-20T09:08:50-06:00"}
	
2025-02-20 09:08:50	
{"account":"sbndci","destPath":"/tmp/vt_***-sbnd_ci","level":"error","msg":"Could not copy source file to destination node","node":"NODE","rsyncOpts":"--perms --chmod=u=r,go=","sourcePath":"PATH","time":"2025-02-20T09:08:50-06:00"}
	
2025-02-20 09:08:01	
{"caller":"runAdminNotificationHandler","level":"error","msg":"Timeout exceeded in notification Manager","time":"2025-02-20T09:08:01-06:00"}
	
2025-02-20 09:08:01	
{"caller":"notifications.runServiceNotificationHandler","level":"error","msg":"Timeout exceeded in notification Manager","service":"sbnd-data-globus_production","time":"2025-02-20T09:08:01-06:00"}
	
2025-02-20 09:08:01	
{"caller":"notifications.runServiceNotificationHandler","level":"error","msg":"Timeout exceeded in notification Manager","service":"hypot-gwms-test_production","time":"2025-02-20T09:08:01-06:00"}
	
2025-02-20 09:08:01	
{"caller":"notifications.runServiceNotificationHandler","level":"error","msg":"Timeout exceeded in notification Manager","service":"dune-ci_ci","time":"2025-02-20T09:08:01-06:00"}
	
2025-02-20 09:08:01	
{"caller":"notifications.runServiceNotificationHandler","level":"error","msg":"Timeout exceeded in notification Manager","service":"annie_production","time":"2025-02-20T09:08:01-06:00"}
	
2025-02-20 09:07:52	
{"experiment":"sbnd","level":"error","msg":"Error pushing vault tokens to destination node","node":"NODE","role":"production","time":"2025-02-20T09:07:52-06:00"}

So there were a bunch of notification handlers that seemed to be waiting on a single bad service. We need to refactor the notification handler so that if there's a timeout with one service, we just go ahead and send the rest of the messages (I suspect the service-level messages are fine, but it's just admin that's not), and then clean up the hanging goroutines properly.

[shreyb]

Talked with @retzkek about this in depth.

There are two ways to really handle this, one short-term, and one long-term.

Short-term:

Rather than have a fan-in, fan-out pattern, it'd be much easier if the main package could create a direct link between a worker.WorkerFunc and the corresponding notifications.ServiceEmailManager. How this would work is a question, because right now, there's a single worker for many services. Maybe the service.Service object could be wrapped into a separate object that also includes the notifications.ServiceEmailManager.ReceiveChan, so that the worker knows which channel to send notifications on.

Then, using maybe a waitGroup, when everything is done, we shut down all the ReceiveChans like we currently do through the notificationsManager in package main. The ideal thing here would be then that we could rip out the entire notificationsManager infrastructure/complication.

Long-term:

The notifications package should really be run in a separate application. token-push should be doing various actions, and then generating errors. Then a separate service could figure out how to alert/notify.