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

The argo-events chart doesn't include an EventBus #840

Open
justinmchase opened this issue Jul 19, 2021 · 30 comments
Open

The argo-events chart doesn't include an EventBus #840

justinmchase opened this issue Jul 19, 2021 · 30 comments
Labels
argo-events enhancement New feature or request on-hold Issues or Pull Requests with this label will never be considered stale

Comments

@justinmchase
Copy link

Is your feature request related to a problem? Please describe.
I installed argo-events through helm but it just doesn't work. I setup my kafka event source and it never connects giving an error about a missing EventBus.

When I manually run kubectl apply -n argo-events -f https://raw.githubusercontent.com/argoproj/argo-events/stable/examples/eventbus/native.yaml then the bus appears and I see it starts connecting.

It really seems like the helm chart should create the EventBus when you install it and just have configuration options to configure it. But the event system seems to not function without it so it probably should be in the chart.

Describe the solution you'd like
I'd like to just be able to helm install argo-events and boom it has everything to function, at least simply, by default.

Describe alternatives you've considered
I can just run more stuff and manually create resources but it is confusing and takes a lot of effort just to even try argo out at all.

Additional context
Additionally, I hate that argo goes into one namespace and argo-events goes into another... Please just put everything under the 1 argo namespace.

@justinmchase justinmchase added the enhancement New feature or request label Jul 19, 2021
@osherlevi
Copy link

Same for me, saw this error:
2021-08-11T10:35:29.296Z ERROR argo-events.eventsource-controller eventsource/controller.go:53 reconcile error {"namespace": "argo-events", "eventSource": "webhook", "error": "eventbus default not found", "errorVerbose": "eventbus default not found\ngithub.com/argoproj/argo-events/controllers/eventsource.Reconcile\n\t/home/runner/work/argo-events/argo-events/controllers/eventsource/resource.go:48\ngithub.com/argoproj/argo-events/controllers/eventsource.(*reconciler).reconcile\n\t/home/runner/work/argo-events/argo-events/controllers/eventsource/controller.go:93\ngithub.com/argoproj/argo-events/controllers/eventsource.(*reconciler).Reconcile\n\t/home/runner/work/argo-events/argo-events/controllers/eventsource/controller.go:51\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\t/home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.7.0/pkg/internal/controller/controller.go:263\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\t/home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.7.0/pkg/internal/controller/controller.go:235\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func1.1\n\t/home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.7.0/pkg/internal/controller/controller.go:198\nk8s.io/apimachinery/pkg/util/wait.JitterUntilWithContext.func1\n\t/home/runner/go/pkg/mod/k8s.io/apimachinery@v0.19.7-rc.0/pkg/util/wait/wait.go:185\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1\n\t/home/runner/go/pkg/mod/k8s.io/apimachinery@v0.19.7-rc.0/pkg/util/wait/wait.go:155\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil\n\t/home/runner/go/pkg/mod/k8s.io/apimachinery@v0.19.7-rc.0/pkg/util/wait/wait.go:156\nk8s.io/apimachinery/pkg/util/wait.JitterUntil\n\t/home/runner/go/pkg/mod/k8s.io/apimachinery@v0.19.7-rc.0/pkg/util/wait/wait.go:133\nk8s.io/apimachinery/pkg/util/wait.JitterUntilWithContext\n\t/home/runner/go/pkg/mod/k8s.io/apimachinery@v0.19.7-rc.0/pkg/util/wait/wait.go:185\nk8s.io/apimachinery/pkg/util/wait.UntilWithContext\n\t/home/runner/go/pkg/mod/k8s.io/apimachinery@v0.19.7-rc.0/pkg/util/wait/wait.go:99\nruntime.goexit\n\t/opt/hostedtoolcache/go/1.14.15/x64/src/runtime/asm_amd64.s:1373"}

I guess that the issue is that the name of the eventbus is different from default, how can we adjust the event source controller?

@kurtbomya
Copy link

kurtbomya commented Aug 12, 2021

I am also surprised to learn that the required CRDs are not generated with something like helm template argo/argo-events since the README.md states that I need to put --skip-crds to prevent the creation of CRDs. But here I am, not putting that flag, and still not getting the CRDs included.

$ helm template argo/argo-events --skip-crds | wc -l
280
$ helm template argo/argo-events | wc -l
280

@mkilchhofer
Copy link
Member

I am also surprised to learn that the required CRDs are not generated with something like helm template argo/argo-events since the README.md states that I need to put --skip-crds to prevent the creation of CRDs. But here I am, not putting that flag, and still not getting the CRDs included.

$ helm template argo/argo-events --skip-crds | wc -l
280
$ helm template argo/argo-events | wc -l
280

The helm template subcommand acts different from helm installor helm upgrade. To include the CRDs in your helm template output, you need to use --include-crds

@reinvantveer
Copy link
Contributor

reinvantveer commented Nov 10, 2021

This behavior is, I agree, unexpected. Just started using this helm chart for event processing, but to my surprise it needs an additional event bus resource for each namespace that is home to event sources and event bindings. Which is not supplied by the helm chart. Is this correct @mkilchhofer ?

@reinvantveer
Copy link
Contributor

@mkilchhofer maybe this escaped your attention. This issue was still waiting for a response.

@reinvantveer
Copy link
Contributor

Could you also re-open the issue? I'm fine submitting a pull request, but what's in it rather depends on the intended strategy for the chart.

@mkilchhofer mkilchhofer reopened this Jan 17, 2022
@argoproj argoproj deleted a comment from github-actions bot Jan 17, 2022
@mkilchhofer mkilchhofer added on-hold Issues or Pull Requests with this label will never be considered stale and removed no-issue-activity labels Jan 17, 2022
@mkilchhofer
Copy link
Member

Oh yeah, this should not have been closed.

But as I don't use argo-events, I cannot help much with this issue. I don't understand what this EventBus.argoproj.io object does and why it's not included in the default manifests.
image

@argoproj argoproj deleted a comment from github-actions bot Jan 17, 2022
@reinvantveer
Copy link
Contributor

reinvantveer commented Jan 17, 2022

Thanks for the action @mkilchhofer. I'll see if I can open a PR for this soon. Argo Events is useless without a EventBus resource. It creates the bus itself where event messages are written to and from.

@mkilchhofer
Copy link
Member

mkilchhofer commented Jan 17, 2022

Hmm I read through the documentation. The architecture picture made me thinking about this issue again.

architecture
The chart seems to be designed to just deploy the controllers.

Besides the EventBus the users need to deploy (depending on their use-case):

  • an event source
    apiVersion: argoproj.io/v1alpha1
    kind: EventSource
    metadata:
      name: webhook
    spec:
      service:
        ports:
          - port: 12000
            targetPort: 12000
      webhook:
        # event-source can run multiple HTTP servers. Simply define a unique port to start a new HTTP server
        example:
          # port to run HTTP server on
          port: "12000"
          # endpoint to listen to
          endpoint: /example
          # HTTP request method to allow. In this case, only POST requests are accepted
          method: POST
  • a sensor:
    apiVersion: argoproj.io/v1alpha1
    kind: Sensor
    metadata:
      name: webhook
    spec:
      template:
        serviceAccountName: operate-workflow-sa
      dependencies:
        - name: test-dep
          eventSourceName: webhook
          eventName: example
      triggers:
        - template:
          # (..)

--> While we could extend the chart to provide this functionality, we must not install anything of EventSource, EventBus Sensor by default.

WDYT @jbehling and @VaibhavPage?

@reinvantveer
Copy link
Contributor

reinvantveer commented Jan 18, 2022

@mkilchhofer I believe resources like EventSource and Sensor are more or less like Workflow resources in the sense that they are fully user-defined and use case dependent: I agree that there is no place for them in the chart. However, the case is different for the EventBus I think.

The event bus is a dependency for the controller deployments. By default, controllers try to connect to an event bus with the unfortunate default name of default (which I believe should be marked as bad practice) and fails to start if not found (see error message by @osherlevi). As is, the events helm chart is an incomplete release in that it does not fully deploy.

Furthermore, the event bus is a dependency for deploying any EventSource (see its spec first field) or Sensor: they get an eventBusName that should match an event bus present in the namespace.

HTH

@mkilchhofer
Copy link
Member

No, also the EventBus CRD is a resource like EventSource and Sensor. Only the eventbus controller talks to this CRD as it is responsible for the lifecycle of the eventbus (upgrade, downgrade, scaling, configuring, etc.).
The dependency you mentioned is exactly this. It watches for these kind of objects and then would create a StatefulSet and other resources to implement the CRD with real (NATS) pods.

From the picture above it looks clear to me that the controllers do not talk with the eventbus.

@reinvantveer
Copy link
Contributor

Aha I see, thanks for the clarification. So, it seems there is a bit of a contradiction in the docs. The architecture picture can be read such that EventBus is a resource like EventSource and Sensor. On the other hand, the install guide marked in yellow in #840 (comment) indicates that the EventBus is part of the installation procedure. From the latter, it would not be unreasonable to imply that the argo-events helm chart comes with step three of the install guide included.

Which brings me to this: it would be helpful if the helm chart either

  1. offers better documentation on why the event bus is not included,
  2. comes with the option to include the event bus, or
  3. just comes with the event bus.

Either way is fine, I think, but in its current state causes confusion, hence this issue. I leave it up to the maintainers to decide on this, but I guess for people starting out with this helm chart, it could be a tremendous time saver to have either option 1 or 2.

Thanks again for your thoughts.

@reinvantveer
Copy link
Contributor

@mkilchhofer can we agree on option 2?

@mkilchhofer
Copy link
Member

Yes, sure. There is already a PR open from the community:
#1109

@reinvantveer
Copy link
Contributor

Ah someone beat me to it! Excellent!

@mgfreshour
Copy link

Since that PR was closed without merging 7 days ago, is there any workaround for this?

@phoenix1480
Copy link

phoenix1480 commented Aug 22, 2022

Hi there, we use ArgoCD, Argo Workflows, and are in process of deploying Argo Events. For Argo Events we are facing same issue as other community members here.

  1. Does the helm chart include the event bus or just a controller to interface with the event bus? It isn't clear if we should be installing the EventBus separately.

  2. If the EventBus could be included in the helm-chart that would be incredibly valuable. I could see it not being added to keep the EventBus fully decoupled. If that is the case, perhaps modifying the documentation to specify that Argo Events requires the EventBus (NATS/JetStream) be installed first.

#2 is okay by us for our needs and a better design practice. That said, getting up and running is made more difficult by this design choice for newcomers like ourselves.

If someone could please confirm that we MUST install the EventBus separately, that would be much appreciated. I admit, our team is stuck atm ;)

@pdrastil
Copy link
Member

pdrastil commented Aug 22, 2022

@phoenix1480 Hi, I've reimplemented the recent version of argo-events and the reason why I haven't added the EventBus even when I saw this issue was because of managing upgrades in multi-tenant environments that can contain multiple EventBus resources.

Example:

apiVersion: argorpoj.io/v1alpha1
kind: EventBus
metadata:
  name: default
spec:
  jetstream:
    version: my-version
    replicas: 3

As you can see in example above the version is tied to configs section in Helm chart below. If there would be a version bump it might upgrade the default bus created / managed by chart but also break other additionally created EventBuses.

configs:
  jetsream:
    version:
      -version: "2.0.2"
        natsImage: <nats-image>:2.0.2
     - version: "2.0.2-alpine"
        natsImage: <nats-image>:2.0.2-alpine
      -version: "2.0.1"
        natsImage: <nats-image>:2.0.2

Second problem would be in referencing customer resource definition that might not exists in the cluster yet. If you want to have preconfigured Argo Events with buses / triggers etc. I would recommend to create your own umbrella chart that will first deploy the controller with configuration as a dependency and then creates all these resources.

@reinvantveer
Copy link
Contributor

If someone could please confirm that we MUST install the EventBus separately, that would be much appreciated. I admit, our team is stuck atm ;)

Yes, Argo Events will go absolutely nowhere without an event bus. The current solution is to create an umbrella chart @pdrastil suggests that includes a manifest like the one suggested in the getting started guide:
https://github.com/argoproj/argo-events/blob/master/examples/eventbus/native.yaml

@reinvantveer
Copy link
Contributor

@phoenix1480 If this helps: I did some tinkering here: https://github.com/Antfield-Creations/gke/tree/main/charts/antfield-argo-events
Strongly advise to review this by the dev team, as it may not suit your needs

@reinvantveer
Copy link
Contributor

@phoenix1480 Hi, I've reimplemented the recent version of argo-events and the reason why I haven't added the EventBus even when I saw this issue was because of managing upgrades in multi-tenant environments that can contain multiple EventBus resources.

This is good to take into consideration for designing updates to the current chart. It should not enable an event bus by default, but it should be made easy to opt into one

@rakhbari
Copy link
Contributor

Sorry I'm a little late to the conversation about this, but I have to disagree with @reinvantveer 's point about "should not enable an event bus by default". Installing an EventBus named default in the argo-events namespace, aside from making things a lot easier on people new to Argo Events, does not prevent you in any way from adding other EventBuses with your own name in other namespaces.

At the very least the values.yaml of the chart should be modified to have a installDefaultEventBus flag (default: false) and the user can then supply true in their values.yaml to have the default EventBus installed in argo-events namespace. Or, if you really want to get fancy, values.yaml can have an array of eventBuses where name and namespace are fields of the array. Either way, IMHO leaving out the ability for creation of the EventBus from the chart is a real hinderance.

@reinvantveer
Copy link
Contributor

reinvantveer commented Dec 22, 2022

@rakhbari thanks for sharing your point of view here. My main consideration was that an update to the chart should not break existing installations, let alone on an EventBus named default - an opt-in installDefaultEventBus flag or comparable should do fine I think. I was unable to find the time to make progress on this issue in last few months, and there has been #1109 before but this hasn't been completed unfortunately. I still hope to be able to find the hours in January to follow this up.

@ghost
Copy link

ghost commented Feb 1, 2023

Hi to all here - getting same on Helm Charts version 2.1.2

Can someone make some clarification of this case ?
As i understand the temp solution of this it is to apply yaml of eventbus native.yaml

kubectl apply -n argo-events -f https://raw.githubusercontent.com/argoproj/argo-events/stable/examples/eventbus/native.yaml

this kubectl apply can make work helm chart as expected ?

@reinvantveer
Copy link
Contributor

reinvantveer commented Feb 1, 2023

Hi @boris-shkarupelov that's right - you deploy an event bus. The "pretty" way of doing this would be to derive your own helm chart from the argo-events chart and put the contents of https://raw.githubusercontent.com/argoproj/argo-events/stable/examples/eventbus/native.yaml in there, so that you have a versioned release, including your event bus. This is exactly why it would be a good idea to include it into the argo-events chart in the first place.

I created something similar here: https://github.com/Antfield-Creations/gke/tree/main/charts/antfield-argo-events
but this uses an old version of Argo Events, no guarantees on it working on newer versions.

@joebowbeer
Copy link

As things stand currently, the installation instructions should be edited to mention that the helm chart does not install an eventbus:

https://argoproj.github.io/argo-events/installation/#using-helm-chart

In addition, -n argo-events --create-namespace should be added to the helm install command line.

@jmeridth
Copy link
Member

@joebowbeer would you like to create a PR for those doc change requests?

@murand78
Copy link

apiVersion: argorpoj.io/v1alpha1

apiVersion: argorpoj.io/v1alpha1

Note the typo, "argorpoj", I noticed only after updating and recreated the crds a lot of times :-)

@joebowbeer
Copy link

@murand78 link to typo?

@murand78
Copy link

sorry, the example in #840 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
argo-events enhancement New feature or request on-hold Issues or Pull Requests with this label will never be considered stale
Projects
None yet
Development

No branches or pull requests