Conversation
|
For unikernel updates notifications, we can run a simple cronjob that calls the endpoint with a validate authentication header: |
|
I've not looked into the code, but I don't understand why there is an HTTP endpoint and an external trigger needed. What is the advantage over "every day at Y o'clock, gather the updates and send mails"? To me it feels like (a) there's some other system needed for the trigger (which can fail), (b) there's a DoS vector (calling every second that endpoint leads to mollymawk doing lots of work and sending lots of mails). |
|
@hannesm I think the CI failure is unrelated to the PR |
yes, this should be fixed by rerunning (ref ocaml/dune#13299 and the workaround ocaml/opam-repository#29219). I restarted the failing builds. |
| let delay = seconds_until_next_midnight () in | ||
| Logs.info (fun m -> m "Next background update in %.0f seconds" delay); | ||
| Mirage_sleep.ns (Duration.of_sec (int_of_float delay)) >>= fun () -> | ||
| Lwt.catch |
There was a problem hiding this comment.
What kind of exceptions do you expect here? I'm curious why using Lwt.catch now, while I thought our code would not expose exceptions (apart from out of memory). In your testing, did you encounter some exception here?
There was a problem hiding this comment.
I didn't encounter any specific bugs but my understanding is that (I could be wrong), with Lwt.async if something fails, the loop won't get restarted automatically and since we are calling external services (builder_web) etc, I'm not sure if everything will always go well, so Lwt.catch was in case something happens, then atleast the loop can run again the next midnight..
| Lwt.pause () >>= fun () -> | ||
| let http = Paf.http_service ~error_handler request_handler in | ||
| let (`Initialized th) = Paf.serve http service in | ||
| Lwt.pause () >>= fun () -> |
There was a problem hiding this comment.
I don't understand these Lwt.pause () -- but I think you discussed them with Romain?
There was a problem hiding this comment.
Without Lwt.pause I was facing a terrible issue where the scheduler might be running but nothing else in mollymawk works, as in, I couldn't open the web platform at all, it will timeout continously.
| port : int; | ||
| sender_email : Mrmime.Mailbox.t; | ||
| mollymawk_domain : string; | ||
| } |
There was a problem hiding this comment.
Is this persisted to disk? Do we need to care about old configurations (and somewhere increase a version number)?
There was a problem hiding this comment.
We do persist this to disk (in case the server is restarted), but I do not think we should care about old configurations or version control them even though what is affected is that all previously sent emails with a different base-url (mollymawk_domain) will lead to dead ends potentially. But I wonder if we should work on getting mollymawk to be able to handle all the different base-urls. Maybe we can setup a trigger, whenever the base-url is changed, an email is sent to all users informing them of this change, and what to do about previously sent emails.
There was a problem hiding this comment.
Just to be sure, there are already running mollymawk instance(s) which have some data persisted to disk. Now you revise the data format, so what will happen with these old data on disk? Will mollymawk fail to start? Will it discard the configuration?
There was a problem hiding this comment.
Our live mollymawk instance was not update with email configuration yet, so we personally will not face this issue but if there is any live instance of mollymawk which was updated then what happens is mollymawk will discard the change, and the user will have to enter the email configuration again for emails to work, but mollymawk will start, just that email configs will be null
|
Looks good, thanks! |
|
I'm not sure what to do here. So, this PR is against another branch - shouldn't this be against the main branch? |
I rebased onto main, and cherrypicked the commits from the other branch that were needed for this. So I think this is fine now. |
|
thanks. |
* update email template * use system email as from email * store domain used by mollymawk * template email for update emails notification * better failed email messages * check email updates and send emails * add mollymawks domain to links * have a scheduler for background checks within mollymawk * remove back ground update endpoint * add lwt.pause * change fields to from for sender_email and base_url for mollymawk domain * formatted code * rename to from and base_url * use duration.of_f * safe to wait for an hour rather than 60 seconds which could lead to spamming users * fix minor bug --------- Co-authored-by: Automated ocamlformat GitHub action, developed by robur.coop <autoformat@robur.coop>
2 participants
This PR runs a background check on all running unikernel instances and then sends emails to users concerned about which unikernels have updates.
Email example