search

livepeer / catalyst-api

MIT License
4 stars 2 forks source link

Proxy Mist triggers from catalyst to catalyst-api and remove dynamic Mist Trigger Setup #1362

Closed leszko closed 2 months ago

leszko commented 2 months ago

I always thought that catalyst-api dynamically sets Mist triggers. This is true, but this config gets overwritten with the default config present here every time MistController restarts of the config file is changed. The only reason it worked in prod is that our dynamic Mist Trigger configuration is exactly the same as the config map configuration.

I think it's super misleading and it led to the issues when I tried to use the standalone catalyst-api, because my config got overwritten and all stopped working at some point.

First of all, I suggest removing this whole dynamic mist triggers setup (to avoid further confusion). Secondly, we need to think how to solve it if catalyst-api is deployed separately.

  1. Option 1: What I implemented in this PR => Proxy each Mist trigger request from catalyst to catalyst-api
  2. Option 2: Monitor Mist triggers and dynamically override them each time the config changes
  3. Option 3: Investigate it further, maybe if we remove the Mist triggers configuration from here it will not override the dynamically set configuration; the reason I didn't start from this option is that I'm afraid we'll come to some corner cases like Mist crashing or something and then we'll end up with Mist with no triggers set up at all
  4. Option 4: Have a separate config map for each catalyst pod (then we can differentiate between triggers in the infra config)
mjh1 commented 2 months ago

Could you link to the block(s) of code for the dynamic triggers setup which in theory could be remove? Or are you saying you've removed those bits already in this PR? This idea of proxying doesn't seem too bad since we're already having to do that for some events right? (implemented in https://github.com/livepeer/catalyst-api/pull/1347)

leszko commented 2 months ago

Could you link to the block(s) of code for the dynamic triggers setup which in theory could be remove? Or are you saying you've removed those bits already in this PR? This idea of proxying doesn't seem too bad since we're already having to do that for some events right? (implemented in #1347)

It's already removed in this PR. Yeah, I think the proxying is ok as a hack 🙃