This is the current Brigade behaviour:
However, considering that a new worker is created for each event, for events with high frequency this behaviour might introduce additional latency and consume cluster resources.
The proposal is to create a long-lived per-project controller, whose job is to pick up events for the project and execute the brigade.js script associated with the project.
Proposed behaviour:
brigade.js script and schedules the jobs to run.Caveats:
This needs further validation, but if possible, could enable Brigade to handle more event-driven scenarios.
Other thoughts:
Is there anything obvious that would prevent this from working?
If this would work, is it something we would want to support?
Thanks!
cc @adamreese @technosophos @krancour @dgkanatsios @vdice @mumoshu @lukepatrick
Possibly related discussion - incremental ETL.
cc @danielepolencic
@radu-matei I wanted this for so long. Happy to help in any way to make this idea a reality!
If I could add one thing it would be that we need an alternative way to expose/collect build logs. AFAIK brig build logs works the same as kubectl logs POD so you need a pod-per-build for it to work, which obviously don't work for this long-running worker.
When I last reverted on it my idea was to use an aggregated apiserver and a CRD instead. That is, you create a Brigade-specific BuildRef custom resource per brigade build, and create your own aggregated apiserver to add a logs REST API endpoint to the BuildRef CRD.
https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/#advanced-features-and-flexibility
Regarding the exposure of logs for non-pod resources, Kubernetes Prow has a feature called ExternalAgentLogs. It works by the user exposing their own HTTP server for exposing logs. But the downside of it is that it isn't secured by K8s RBAC. In contrast, the aggregated API server can be secured by RBAC.
this would not replace the current supported scenario, and would be a new component.
Agreed.
I'd also like it if we can (eventually) switch which controller to use at the build level. For example specifying the controller name when installing the controller with Helm and setting controllerName: the-ctlr-name when creating a brig build may work.
the init container workflow would no longer be possible, and jobs would have to have a separate step for pulling assets
Can I ask why is that so? I was thinking that, as long as the new controller passes commit refs and branches to the brigade.js processes run inside the controller, init containers should just work as today.
Just a quick note.
This design might require some sort of persistent storage as you need to buffer the incoming events somehow (the same isn't true with the existing design).
@mumoshu - yeah, my comment was mainly related to how right now there is an init container to the worker - but I agree, we should keep the option to have one for jobs.
@danielepolencic - starting with #966 you can attach an existing Kubernetes volume to a job - is there anything else that you would expect from that functionality?
Thanks!
If I could add one thing it would be that we need an alternative way to expose/collect build logs. AFAIK
brig build logsworks the same askubectl logs PODso you need a pod-per-build for it to work, which obviously don't work for this long-running worker.
The only thing that would be different related to logs would be the worker logs - job logs should be accessible in exactly the same way they are right now, right?
starting with #966 you can attach an existing Kubernetes volume to a job - is there anything else that you would expect from that functionality?
I was thinking something different.
With the current design, every time an event is created, a new worker is spawned. While largely inefficient, it's quite straightforward to handle: 1000 events -> 1000 workers. No state.
With the proposed design, the controller can handle multiple events. However, the controller has to spend time processing each incoming event.
If the controller takes 1 second to process an event, you should make sure that events produced at a higher rate (e.g. 1/2 second) are queued up or they could be lost.
In the proposal, I think the new controller is stateful.
Oh, ok - missed the part about buffering events.
This raises two questions about the design of this component:
brigade.js script - but in theory, I could create a job and set its timeout to any value I want. How would we handle these scenarios?Not an expert (and I'm not familiar with brigade's architecture), but the design reminds me of the actor model.
Usually, each actor has its own queue and messages are queued up by default. When the actor feels like it, it can consume messages from the queue.
In this case, if each controller has a queue then it doesn't matter how long the script takes to process the events.
FWIW, I'm thinking that this can possibly end up as a set of brigade-utils-like libraries to write a long running K8s controller in JS.
The controller will run brigade.js AND/OR anything had been running in worker's init containers and Job containers are run either "in process", or in ephemeral containers.
For scaling the controller according to load, the controller written in js may expose metrics like "average build queue duration" that is the duration from the time a job is queued(job secret is created), to the time the job is run by the controller.
Re: https://github.com/brigadecore/brigade/issues/977#issuecomment-522605007 I think every controller instance should watch all the builds(secrets) and try to optimistic-lock queued job before running it. It would make jobs not lost at high-rate. It would also scale almost horizontally if we could separate queues(=namespaces where build secrets stored?) per controller instances.
I'm still interested in this, and I basically agree with @danielepolencic's call that this looks similar to the actor model where each actor is a "custom" brigade-controller that is written in JS or any language.
Regarding the design, I would propose two variants:
In this variant, we'll maintain the standard model of Brigade deployment that a brigade cluster is shared across one or more projects.
Must-have:
./brigade-controller.js per Git repository which defines the custom brigade controller per repositoryNice-to-have:
defaultScript for the brigade project that is used instead of the brigade-controller.js when missingTBD:
In this variant, you'd deploy a Brigade cluster per a Git repo that requires a custom Brigade controller.
Must-have:
An additional field controller.* to specify a custom brigade controller by container image, that would looks like worker's.
Example:
## worker is the JavaScript worker. These are created on demand by the controller.
controller:
registry: brigadecore
name: brigade-controller
# command:
serviceAccount:
create: true
name: brigade-worker
# tag:
# pullPolicy: IfNotPresent
Nice-to-have:
defaultScript field added to the controller setting per brigade cluster proposed above. This in combination with controller.tag pointed to the base image for custom brigade controller, you can customize the custom controller by just tweaking the defaultScript in a helm chart value.Regardless of which variant we go for, we'd need the followings:
brigadier or implement a brigadier-like nodejs module for writing custom brigade controllersbrig build logs won't support logs emitted by custom brigade controllersbrigade controller logs <PROJECT NAME> for the variant 1, or brig controller logs <NO ARG> for the variant 2 to obtain controller logsMaybe it's better to start with the variant 2 and then evolve to the variant 1 as the latter requires more work and works done for the former can reduce cost implementing the latter.
Unless there are serious objections, I'd like to tag this as a proposal for 2.0. While it's within the realm of possibility that this could be accomplished in 1.0, I think there's a good possibility we'd have to make some major concessions to accommodate what is essentially very different architecture from what Brigade employs today. If we consider this in the 2.0 timeframe, we give ourselves the greater latitude to design _for_ this with fewer legacy constraints.
For anyone concerned about what this means in terms of a time-frame, maintainers are gearing up for 2.0 planning at the end of the month.
It's taken me some time to fully absorb the many things proposed throughout this thread. While I'm certain this is going to be a topic of discussion at the upcoming planning session, I do also want to start assembling some thoughts here...
My first thought is that combining the responsibility of a controller and a worker into a single component, even if only on an opt-in basis, comes with a number of challenges-- some of which have been acknowledged previously in this thread. There are others, however, that have been overlooked.
One that has been acknowledged is that worker logs wouldn't be available, unless major changes are made to how we aggregate, store, and index logs. That is (potentially) on the table for 2.0 already, but even so, there's a lurking difficulty here. Log collecting/forwarding agents (like fluentd, for example) collect logs on a _per-container_ basis. If the agent has to look at every line in a given container's logs individually to tag/route on the basis of which "virtual worker" produced the line, that has the potential to severely impede the logging infrastructure's throughput.
Stepping back, the particular problem I describe above can be generalized: the isolation that results from each worker running in its own pod is an asset. The notion that a fatal error experienced by one "virtual worker" could knock out other in-progress workers isn't comforting.
Another difficulty: Under the current architecture, build status is determined solely by the status of its worker and the (running or completed) worker pod is the _only_ record of the worker's existence. i.e. Without a worker that exists as its own pod, _a build has no status_. My intuition says that some of that may change in 2.0, but to what extent isn't clear-- again, much of this is still to-be-discussed. I raise this only to point out that nothing proposed previously in this thread addresses the major architectural changes that would be required to overcome builds having no status.
Perhaps the most problematic issue that I see with combining the function of a controller and worker, even on an opt-in basis, is that it complicates the notion of a custom worker-- something which I have been a big fan/proponent of. Suppose, for instance, that you wish to opt-in to one of these long-running controller-worker's, but _also_ need customizations. You're left implementing both a worker _and_ controller instead of just the worker. Besides being a higher bar to meet, it makes me uncomfortable. If I am the operator/admin of a Brigade cluster, I wouldn't want people deploying their own controllers. It is too risky. What happens to the rest of the Brigade cluster when they've made a mistake in their controller logic? (Make a mistake in a custom worker-- screw up your project. Make a mistake in a custom controller-- screw up _other_ projects. I realize the fallout from that can potentially be mitigated by allowing projects to exist in their own namespaces-- which has also been proposed-- but I'll come back to the fact that that's _another_ major architectural change that _could_ happen, but is still pending discussion.)
Now... I don't want to look like I'm being overly critical of this idea. I'm just trying to highlight some challenges that I know would exist so that they're nor overlooked as discussion continues. i.e. Please don't interpret this as strong dissent. It is merely the voice of caution.
I would, however, like to propose a couple alternatives that could work on a case-by-case basis _starting today_.
I'll sound like the worst possible proponent of my own project by saying this, but _maybe_ don't use Brigade. If this proposal is being put on the table because a new pod-per-worker is introducing too much latency or impeding throughput, then it should at least be considered that Brigade might not (at least at present) be the ideal approach for use cases requiring great speed or involving high volumes. (fwiw, I'd love if 2.0 heads in a direction that gives us more confidence in dealing with such use cases.) Indeed, with a small fraction of the time/effort required to bend Brigade to accommodate such use cases, one could quite easily whip up something controller-like that receives events on whatever channel(s) (
If you want to cut pods out of the equation to decrease latency or increase throughput, there's _already_ a way of doing this in Brigade, but it involves doing something that I think has been unfairly / unnecessarily stigmatized. The default worker is great for _not actually doing any work_, but executing a workflow/pipeline instead, where all the work is distributed to jobs that each exist in their own pods. But one could always create a custom worker that does the work _directly_. This would be much faster and it would work without a single change to Brigade.
fwiw, I'm going to open a separate issue/proposal for 2.0 aimed at de-stigmatizing the approach of workers that actually work and also reducing the barriers to (complexity of) creating custom workers so that this approach becomes all-the-more easy in the future when a use case calls for it.
Amending my advice on existing workarounds-- if you're comfortable with JavaScript, you don't even need a custom worker-- you can simply write a brigade.js that does work without launching jobs to do it.
Having the controller delegate to the worker makes sense. Perhaps this functionality could be achieved by not destroying the worker pod when a task is complete. The worker could have some timeout where it scales down if it doesn't receive a new event after say 30 minutes (configurable). Then the controller can choose to delegate an event to this idle pod rather than spinning up a new one.
I believe this issue has been rendered moot by #1051 and, subsequently, by the 2.0 proposal.