Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Controller should create EventListener pod on creation #36

Closed
bobcatfish opened this issue Jul 23, 2019 · 5 comments
Closed

Controller should create EventListener pod on creation #36

bobcatfish opened this issue Jul 23, 2019 · 5 comments
Assignees
Milestone

Comments

@bobcatfish
Copy link
Collaborator

Expected Behavior

When an EventListener CRD is created, the triggers controller should respond by making an instance of running container that can be invoked with CloudEvents.

Eventually this running container would expose a Knative eventing addressable endpoint which could be invoked with CloudEvents and would create resource instances in response, but in the context of this issue, the goal is just to:

  • Start the container on EventListener creation
  • Delete the container on EventListener deletion (thought: should it be allowed to finish whatever it's doing before it finishes? probably! i guess this depends on the design of the running container!)

Actual Behavior

Creating an EventListener current does nothing :D

Additional Info

@bobcatfish writing this issue does not know the exact form that this running container should take. She knows it should be a container inside a pod, but is not sure if it should be a stateful set, deployment, or something else :D Anyone with more context (@iancoffey @ncskier @vtereso ) feel free to replace this with something definitive or whoever takes on the issue can decide!

Other thoughts:

  • This means adding an image which we can build for the running EventListener itself!
  • The logic in this image should have tests
  • The image should have a README in cmd
@vtereso
Copy link

vtereso commented Jul 23, 2019

From my understanding and according to the docs:

If an application doesn’t require any stable identifiers or ordered deployment, deletion, or scaling, you should deploy your application with a controller that provides a set of stateless replicas. Controllers such as Deployment or ReplicaSet may be better suited to your stateless needs.
Limitations

I think Deployment makes the most sense since there isn't really anything stateful to the listener pods?

@ncskier ncskier self-assigned this Jul 23, 2019
@ncskier
Copy link
Member

ncskier commented Jul 23, 2019

@iancoffey do you have an opinion? Was there an advantage to using a StatefulSet for the TektonListener?

@iancoffey
Copy link
Member

iancoffey commented Jul 24, 2019

iirc, the reasons I chose statefulset are the Ordered, graceful deployment and scaling and Ordered, automated rolling updates properties meant it less likely two listeners of different versions be exposed at once. That seemed like a big potential problem at the time.

https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#using-statefulsets

I dont have a strong opinion and more or less just rolled with it. The set approach always worked fine but not moreso than a Deployment, in the case that we dont care about the deployment orderings (or I just misunderstand them)

@ncskier
Copy link
Member

ncskier commented Jul 24, 2019

Ok, thanks Ian. I'm not extremely knowledgable about those situations where we might want a StatefulSet over a Deployment, but it seems like a Deployment will work well for us right now. Down the line, if we want to change it to a StatefulSet, we can.

@dibyom dibyom added this to the Triggers 0.1 milestone Aug 15, 2019
@ncskier
Copy link
Member

ncskier commented Aug 15, 2019

Sorry, I should have closed this earlier from #43

@ncskier ncskier closed this as completed Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants