search

knative-extensions / eventing-natss

NATS streaming integration with Knative Eventing.
Apache License 2.0
40 stars 41 forks source link

Support Nats JetStream #145

Closed zhaojizhuang closed 2 years ago

zhaojizhuang commented 3 years ago

Problem Related to quesion in knative-users GoogleGroups

We are now using nats streaming server. And Nats deprecated nats streaming server. Instead they suggest using JetStream.

So we should support jetstream in eventing-natss's dispatcher,

This is a long-term task, we will gradually replace it by deprecating supporting nats streaming server, maybe supported at about 0.26 version or later

steps

Persona: Which persona is this feature for?

People who use jetstream for Event Channel

Exit Criteria A measurable (binary) test that would indicate that the problem has been resolved.

Time Estimate (optional): How many developer-days do you think this may take to resolve?

maybe 2 or 3 days？

Additional context (optional)

update nats client package from stan.go to nats.go https://github.com/nats-io/nats.go

zhaojizhuang commented 3 years ago

cc @vaikas

vaikas commented 3 years ago

Is it possible to support both (maybe controlled by flag for the reconciler?) on which one we use, then we could deprecate the old one and remove it after couple of releases if you want to?

zhaojizhuang commented 3 years ago

It's ok to support both, and we can support both for 3 releases before remove the old one.

zhaojizhuang commented 3 years ago

/assigin

devguyio commented 3 years ago

I believe this requires a new CRD since the current CRD is NatssChannel, and now you'll probably want a JetStreamChannel or NatsJetStreamChannel or something. There's gonna be very little code sharing so I'd just deprecate the NatssChannel and in parallel work on JetStreamChannel . One thing to consider, is renaming this to repo to eventing-nats so that it hosts all NATS related eventing components, NATS/JetStream Channel/Source/Sink/Broker.

npu21 commented 3 years ago

hi All, I notice that eventing natss still using 0.11 as https://github.com/knative-sandbox/eventing-natss/blob/main/config/broker/natss.yaml#L69 However, nats-streaming has 0.22 and more newer version as https://hub.docker.com/_/nats-streaming?tab=tags&page=1&ordering=last_updated Do we need update first before moving on?

zhaojizhuang commented 3 years ago

@npu21 Of course we can，by the way, are you using eventing-natss in production now?

dan-j commented 3 years ago

Are there any published docker images to try this out sooner? We're currently building a new system based off JetStream and would love to get in on the Knative support as an early user. We're still in development so "production-ready" isn't really an issue for us just yet.

If there are any significant outstanding issues to using it and can split the work up I'd be willing to contribute something too. I haven't contributed to Knative before but keen to learn (I've written my own operators and other in-depth k8s stuff in the past)

zhaojizhuang commented 3 years ago

@dan-j Not yet, We plan to officially support the alpha version in release 0.26

If there are any significant outstanding issues to using it and can split the work up I'd be willing to contribute something too. I haven't contributed to Knative before but keen to learn (I've written my own operators and other in-depth k8s stuff in the past)

Great, Contributions are welcome！ cc @devguyio @wallyqs

dan-j commented 3 years ago

I spent a bit of time looking into the implementation of this and have a few points I'd like to raise for discussion. Keep in mind I'm still getting my head round the whole Knative ecosystem so I may have made some assumptions which are incorrect.

  1. All messages are sent to the K-ORDERS stream which subscribes to all K-ORDERS.* messages. Should this not be configurable within the NatsJetstreamChannel spec?
  2. Stream configuration - there are many options for creating a stream, would it be a good idea to allow users to control this configuration?
  3. What about secured NATS clusters? There's no way to configure user credentials (ideally from a secret) or TLS settings.
  4. Multi-tenancy - This kind of ties in with 3., but two streams of the same name can exist across different accounts (determined by the credentials you use).
  5. If you created multiple NatsJetstreamChannels either with the same name in different namespaces or different names in the same namespace, what would happen?

I think there's quite a lot of missing for this to be at feature parity with the Kafka implementation. Is this considered a goal?

I've not looked into the NATS Streaming implementation, but if this has the same constraints, would starting a new project with a focus on JetStream be easier? Following the kafka implementation as a guide, rather than NATS Streaming?

dan-j commented 3 years ago

Ok, I've done a bit more investigation into this, and also familiarised myself with the sample-controller repo on how to bootstrap a new CRD into knative-eventing

I propose the following:

Things to consider:

Side note: has anyone considered implementing Jetstream as a Source as well as a Channel? This would be good for bringing in existing Jetstream users into Knative.

zhaojizhuang commented 3 years ago

@dan-j thank you for your suggestion，just came back from vacation

  1. A new NatsJetstreamChannel which creates a new stream named KN.. which allows configuring: publisher options subscriber options stream options NATS config (or this could come from a ConfigMap like how Kafka works)

For Streaming configuration, yes, we can add them in the NatsJetstreamChannel's spec, as I talked with @vaikas in the PR before（This exactly is why we add NatsJetstreamChannel's reason）:

I think it is necessary to join a new channel. For example, users can specify streamName(hasn't been added to the natsjetstreamchannel) in the channel and other features（maybe more than I can see now） that NATSS doesn’t have. And global configuration for JetStream configed in configmap

  1. For "receiver" deployment and "dispatcher" deployment, I agree with @antoineco ，just add the features in dispatcher.

And I think there is no need to create a new repo, for we are changing eventing-natss to eventing-nats, which includes both jetstream and nats streaming ( mark deprated )

And welcome to grow eventing-nats together!

dan-j commented 2 years ago

Hello all. Just an update from our earlier conversations. We have some capacity to dedicate some time to this and will probably be planning it into our sprint starting next week. Haven't looked any more into the current implementation since my previous comments, so I don't know how long this will take, but thought it would be worth providing an update and let you know this hasn't been forgotten about.

lionelvillard commented 2 years ago

Wonderful!! Thanks for the update @dan-j.

zhaojizhuang commented 2 years ago

Great!!!@dan-j

dan-j commented 2 years ago

I've added a PR with a spec proposal. It would be great if people could review before I start writing code. Thanks!

zhaojizhuang commented 2 years ago

Happy to review @dan-j

dan-j commented 2 years ago

I've been thinking more about this and I don't think I like the fact we're configuring the NATs URL and credentials in the CRD.

Different URLs/credentials means managing separate connections, this makes the code harder to write (and then maintain).

I'm now thinking that, at least for the first implementation, we configure the URL and credentials from a ConfigMap.

However, this isn't going to work long-term for me personally. As a second release we may implement something similar to kafka's distributed approach where you can have a dispatcher per namespace, and each dispatcher is configured with a different ConfigMap.

Approaching the problem this way makes releasing new features easier and we can do it incrementally.

zhaojizhuang commented 2 years ago

@dan-j I comment in PR #255

dan-j commented 2 years ago

Still making progress with this, I'm in the middle of testing my controller changes and will be moving onto the dispatcher in due course.

One question, do we mind making changes to v1alpha1/NatsJetStreamChannel, or should we create a new v1alpha2 version? My gut instinct is change v1alpha1 since it's not actually been released yet, but wanted to double check.

zhaojizhuang commented 2 years ago

@dan-j It's better to make changes to v1alpha1/NatsJetStreamChannel

mericano1 commented 2 years ago

hi all, what's the latest on this ? is there something we could start using on a newly installed nats server?

zhaojizhuang commented 2 years ago

@mericano1 Now the alpha version of NatsJetStreamChannel is supported, Optimization and refactoring in progress by @dan-j

dan-j commented 2 years ago

I've actually got Friday blocked out to do more work on this. Although it's been a good couple of months since I last looked at it so I don't really know how much longer it's going to take. My company is getting closer to actually needing this functionality so the priority from my perspective is increasing.

There should hopefully be some updates to the PR over the weekend so check back next week.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

zhaojizhuang commented 2 years ago

/remove-lifecycle stale

github-actions[bot] commented 2 years ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

mfreeman451 commented 8 months ago

/reopen

knative-prow[bot] commented 8 months ago

@mfreeman451: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to [this](https://github.com/knative-extensions/eventing-natss/issues/145#issuecomment-1903335549): >/reopen Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.