triggers: Proposal: Make triggers less generic and more coupled to Pipelines

TriggerTemplates are very generic and can (theoretically) be used to create anything. We initially made them very generic on purpose b/c this felt like a better design: less coupling = better!

However, by making them less generic, the interface is pretty verbose and we lose out on validation (i.e. we have no idea what you want to create until you try to create it).

We’ve also started to talk about co-ordinating releases https://github.com/tektoncd/plumbing/issues/413 and it seems like the releases that it makes the most sense to combine are pipelines + triggers. We’ve also debated how to display what versions of triggers + pipelines items in the catalog are compatible with, and what the CLI and dashboard will be compatible with.

I’d like to propose: what if we release Triggers + Pipelines together? If we’re doing that, what if we couple them together as well? And if want to take this to its extreme, we could even move them into the same repo (this would simplify things such as consuming updates from knative/pkg for our controllers).

We don’t have to go that far just yet, but in the near term I’d like to propose an update to TriggerTemplates that is more coupled to Pipelines. If we go this route, we might even want to rename the type, and deprecate TriggerTemplates.

So my proposal is: replace TriggerTemplates with something else. I’m not sure what to call it, it’s not exactly a template, but it would be used to create instances of PipelineRuns, TaskRuns and Runs - maybe it’s a “factory”? ( @ImJasonH you might have some naming ideas here) “recipe” even? PipelineRecipe <-- that’s my strawman proposal but totally open to anything better.

Here’s what it could look like (based loosely on https://github.com/tektoncd/plumbing/blob/0ccc386dc6247a4b6e0669d8ab2b11a3f5a81d51/tekton/ci/plumbing-template.yaml):

# There's definitely a question of what api group this is part of: it's so coupled
# to Pipelines that maybe it makes sense to have it in the pipelines api group?
# If we DO want to go as far as combining the repos, we might want to use the same
# api group for both triggers and pipelines
apiVersion: triggers.tekton.dev/v1beta1
kind: PipelineRecipe
metadata:
  name: tekton-plumbing-ci-pipeline
spec:
  params:
  - name: buildUUID
  - name: package
  - name: pullRequestNumber
  - name: gitRepository
  - name: gitRevision
  - name: gitCloneDepth
  - name: gitHubCommand
  pipelineruns:
  -  metadata:
        name: tekton-noop-check-$(uid)
        annotations:
          tekton.dev/gitURL: "$(r.params.gitRepository)"
     serviceAccountName: tekton-ci-jobs
     pipelineRef: # I made all the "spec" fields top level - could keep spec, but what im trying to show is that PipelineRecipe would be more aware of Pipelines
       name: tekton-noop-check
     params:
       - name: passorfail
         value: pass
       - name: gitCloneDepth
         value: $(params.gitCloneDepth) # "r.params" for recipe? i dunno :D - or we might not need it since in the spec embedding case we'd know more about the pipeline and it's params
       - name: fileFilterRegex
         value: "(tekton-demo)"
  # We'd have explicit sections for Pipeline types we know about
  taskruns:
    ...
  runs:
    ...

In this version, we’ve removed a lot of the templating ability you have in TriggerTemplates, e.g. you can no longer use templating to control the names of fields, only the values. I think that might be a good thing, but we also don’t have to go all the way to this extreme.

What do you think???

About this issue

Original URL
State: open
Created 4 years ago
Reactions: 4
Comments: 24 (14 by maintainers)

Most upvoted comments

I definitely liked the genericity of trigger templates.

I see the advantages of being able to validate the template content but I also hoped that it would let me create any kind of resource.

Would it make sense to support both ?

eddycharly on Aug 6, 2020

I put my vote down for keeping the existing generic template/binding/eventlistener functionality as is. I really like the fact that I can “cause” the creation of any Kube object with any sort of internal or external event. The separation of the EL, from definition of what’s to be created (TriggerTemplate), and then having the binding glue things into TrigerTemplate was brilliant and I certainly wouldn’t want that changed.

I don’t see it as too verbose. A bit of a steep learning curve when you’re first trying to get your head around it, but when you understand what it’s trying to achieve and how powerful it is, you get it immediately. Some better documentation can help with the learning curve. Also, automation tooling can make the job of creating them easier. But I wouldn’t want to devolve the model in any way because of these reasons.

we lose out on validation (i.e. we have no idea what you want to create until you try to create it).

@bobcatfish I’m not sure what you mean by this. TriggerTemplate already contains resourcetemplates.kind and with that knowledge the rest of the fields can be precisely validated. Or am I missing something?

I realize this is way after the fact, but my input on the name TriggerTemplate is that it’s not really a “template” for the trigger, rather a template for the TARGET object (resource) of the trigger. So I thought it would be better named as TriggerTargetTemplate. But like I said, that ship has already sailed, so I’m happy to live with TriggerTemplate.

release Triggers + Pipelines together? If we’re doing that, what if we couple them together as well?

My 2-cent’s worth of advice on this… I’m generally against mono-repos. Keep the code bases as small as possible, singular function in mind, low coupling through interfaces, etc. Compatibility between projects can be easily managed with an addition of a compatibility map that lists the project and a minimalVersion string that can be examined at runtime and proper error messages produced if there’s an attempt to run it with a lower than minimalVersion.

PipelineRecipe

I really don’t see a need for this or what it also could be called as PipelineTemplate. From everything I can tell, modeling a Pipeline as has been done and then injecting runtime args into it and modeling that as a PipelineRun run is plenty enough.

rakhbari on Aug 6, 2020

I like the idea of a pipeline recipe as a blueprint for creating pipeline run, task run and runs for a particular pipeline, a resource that contains the config data. However this doesn’t need to be part of triggers. Trigger would be one way to invoke the API to create instances of pipeline runs based on the recipe, but the same need exists for CLI or Tekton Dashboard.

For example with tkn, I would be able to run tkn pipeline start and point to a recipe, instead of numerous arguments to configure workspaces, service accounts, pipeline resource, etc.

siamaksade on Aug 4, 2020

@ImJasonH We (eBay) have greatly extended our internally installed/maintained Kubernetes clusters with several CRDs. A few, having to do with recording of “quality gates” need to be created when certain events occur in our Github Enterprise servers. The EventListener mechanism is the absolute perfect vehicle for these use cases.

So the versatility of the triggers/binding/EL mechanism can’t be overstated. Regardless of what sort of optimization is made to create a more pipeline-centric mechanism, I for one would very much advocate for that to be an addition and keeping the existing triggers/binding/EL functionality as is.

rakhbari on Aug 7, 2020

hopefully this wouldn’t involve a major breaking change, i.e. refactoring of existing trigger config setups running in folks prod systems to comply w/ this change. Works fine for me as is, and today it makes intuitive sense on how it fits together. Why change it? If “recipies” needs to be added maybe do it separately, whenever I hear of a “recipe” i think of “wizardy” types of things which I avoid. The less coupling the better. The name choice of “trigger” to me seems perfect. The generic and totally open way triggers is currently implemented is exactly why I like it, I can do all sorts of very custom things with it.

stick w/ more generic and less coupled.

you can no longer use templating to control the names of fields, only the values

nooo!

bitsofinfo on Aug 6, 2020

Oh I don’t have a strong opinion or any signal either way. I just thought while we’re changing the API we can take the opportunity to reassess other decisions if we thought it could be helpful. This could also be a distraction! 😅

imjasonh on Aug 4, 2020

whenever I hear of a “recipe” i think of “wizardy” types of things which I avoid. The less coupling the better

Totally: the less magic the better! And that’s pretty much exactly why we made the design so generic in the first place.

The generic and totally open way triggers is currently implemented is exactly why I like it, I can do all sorts of very custom things with it.

And @bitsofinfo, if we decide to pursue this (if!), we could decide that we want to have BOTH the more coupled type and continue to support the ultra generic TriggersTemplates as well even into beta.

Are you able to provide more detail about how are you are using the more generic features TriggersTemplates?

bobcatfish on Aug 6, 2020