argo-helm icon indicating copy to clipboard operation
argo-helm copied to clipboard

The argo-events chart doesn't include an EventBus

Open justinmchase opened this issue 2 years ago • 30 comments

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 avatar Jul 19 '21 17:07 justinmchase

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/[email protected]/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/[email protected]/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/[email protected]/pkg/internal/controller/controller.go:198\nk8s.io/apimachinery/pkg/util/wait.JitterUntilWithContext.func1\n\t/home/runner/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:185\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1\n\t/home/runner/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:155\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil\n\t/home/runner/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:156\nk8s.io/apimachinery/pkg/util/wait.JitterUntil\n\t/home/runner/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:133\nk8s.io/apimachinery/pkg/util/wait.JitterUntilWithContext\n\t/home/runner/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:185\nk8s.io/apimachinery/pkg/util/wait.UntilWithContext\n\t/home/runner/go/pkg/mod/k8s.io/[email protected]/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?

osherlevi avatar Aug 11 '21 12:08 osherlevi

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

kurtbomya avatar Aug 12 '21 16:08 kurtbomya

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

mkilchhofer avatar Oct 14 '21 12:10 mkilchhofer

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 avatar Nov 10 '21 10:11 reinvantveer

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

reinvantveer avatar Jan 17 '22 08:01 reinvantveer

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.

reinvantveer avatar Jan 17 '22 08:01 reinvantveer

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

mkilchhofer avatar Jan 17 '22 08:01 mkilchhofer

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.

reinvantveer avatar Jan 17 '22 09:01 reinvantveer

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?

mkilchhofer avatar Jan 17 '22 12:01 mkilchhofer

@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

reinvantveer avatar Jan 18 '22 11:01 reinvantveer

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.

mkilchhofer avatar Jan 18 '22 13:01 mkilchhofer

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 https://github.com/argoproj/argo-helm/issues/840#issuecomment-1014267141 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 avatar Jan 20 '22 20:01 reinvantveer

@mkilchhofer can we agree on option 2?

reinvantveer avatar Feb 22 '22 06:02 reinvantveer

Yes, sure. There is already a PR open from the community: https://github.com/argoproj/argo-helm/pull/1109

mkilchhofer avatar Feb 22 '22 06:02 mkilchhofer

Ah someone beat me to it! Excellent!

reinvantveer avatar Feb 22 '22 12:02 reinvantveer

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

mgfreshour avatar Mar 17 '22 22:03 mgfreshour

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 ;)

phoenix1480 avatar Aug 22 '22 16:08 phoenix1480

@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.

pdrastil avatar Aug 22 '22 18:08 pdrastil

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 avatar Aug 23 '22 12:08 reinvantveer

@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 avatar Aug 23 '22 12:08 reinvantveer

@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

reinvantveer avatar Aug 23 '22 12:08 reinvantveer

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.

rakhbari avatar Dec 22 '22 00:12 rakhbari

@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 https://github.com/argoproj/argo-helm/pull/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.

reinvantveer avatar Dec 22 '22 09:12 reinvantveer

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 ?

ghost avatar Feb 01 '23 12:02 ghost

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.

reinvantveer avatar Feb 01 '23 12:02 reinvantveer

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.

joebowbeer avatar May 16 '23 09:05 joebowbeer

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

jmeridth avatar May 16 '23 13:05 jmeridth

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 :-)

murand78 avatar Aug 29 '23 13:08 murand78

@murand78 link to typo?

joebowbeer avatar Aug 29 '23 13:08 joebowbeer

sorry, the example in https://github.com/argoproj/argo-helm/issues/840#issuecomment-1222787247

murand78 avatar Aug 30 '23 08:08 murand78