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

[QE] Allow override transport for wathola sender with plugin system #4812

Closed
cardil opened this issue Jan 29, 2021 · 14 comments · Fixed by #4815
Closed

[QE] Allow override transport for wathola sender with plugin system #4812

cardil opened this issue Jan 29, 2021 · 14 comments · Fixed by #4815
Assignees
Labels
area/test-and-release Test infrastructure, tests or release kind/feature-request lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@cardil
Copy link
Contributor

cardil commented Jan 29, 2021

Problem
Currently, wathola sender, a tool used in eventing upgrade tests, can send events only using HTTP POST request to a specified URL. That's okay for most of the tests, but in some cases it's lacking. Mostly for testing various Knative sources. In those tests we would like to send events directly to a given system like for ex.: Kafka's topic to be consumed by source under test.

Persona:
Developer

Exit Criteria
A set of Golang interfaces should be created, for sending events, and for transport configuration. The configuration should be added to allow override default HTTP Post transport with custom one provided.

After fixing this issue, it should be possible to use wathola tool to write tests for knative-extensions/eventing-kafka#67

Time Estimate (optional):
2d

@cardil
Copy link
Contributor Author

cardil commented Jan 29, 2021

/area test-and-release
/assign

@knative-prow-robot knative-prow-robot added the area/test-and-release Test infrastructure, tests or release label Jan 29, 2021
@cardil cardil changed the title [QE] Wathola sender override transport with plugin [QE] Allow override transport for wathola sender with plugin system Jan 29, 2021
@zhongduo
Copy link
Contributor

You might want to check out the cloudevents sdk-go, I think it has some binding and transport supports.

@cardil
Copy link
Contributor Author

cardil commented Jan 29, 2021

@zhongduo right, there's gochan protocol that I can use to send an event to a different place, but that will also require some change in sender code to allow influencing that from outside packages.

@grantr grantr added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Feb 1, 2021
@grantr grantr added this to the Backlog milestone Feb 1, 2021
@vaikas
Copy link
Contributor

vaikas commented Feb 1, 2021

@slinkydeveloper do we think this might be functionality for the eventshub since it's using the cloudevents, etc.?

https://github.com/knative-sandbox/reconciler-test/tree/master/cmd/eventshub

@slinkydeveloper
Copy link
Contributor

do we think this might be functionality for the eventshub since it's using the cloudevents, etc.?

Maybe? I'm not really sure what's the requirement here.

I do actually wonder why we don't use eventshub (which is meant to be the one stop for all utilities to send/receive/transform events) for upgrade tests

@vaikas
Copy link
Contributor

vaikas commented Feb 2, 2021

@slinkydeveloper that's why I wanted to loop you in here to see if we can utilize eventshub for all the needs. I wasn't sure if maybe it was missing something. looks like we want to maybe have something like kafka sender, and for example in knative-sandbox/eventing-rabbitmq we have a sender that knows how to send native rabbitmq messages to exercise the source.

@cardil
Copy link
Contributor Author

cardil commented Feb 3, 2021

@slinkydeveloper I'm not aware of this eventshub thing. Can you provide some knowledge links, so I can read more what is the purpose of it?

On the other hand I'll try to provide some background here for you what upgrade tests are doing with their wathola tool.

The overall picture can be seen in https://github.com/knative/eventing/blob/master/test/upgrade/README.md#probe-test paragraph.

TL;DR - it has multiple distributed services, that continually (with constant interval) sends numbered events, receive them, and verify that all of them were propagated at least once. It also reports doubles. Recently, @zhongduo added capability to detect and report unavailability periods.

So, it is a tool written to assert continual, proper, operation of eventing system.

As of now this tool utilizes cloudevents HTTP sender. This issue and draft PR #4815 is to expand that capability to enable use cases @vaikas mention: kafka native sender, rabbitmq native sender, etc. Those would allow us to reliably tests various Knative sources.

@slinkydeveloper
Copy link
Contributor

slinkydeveloper commented Feb 4, 2021

@cardil eventshub is able to perform the same tasks, with the difference that today it doesn't record the aggregation of events, but it record them one by one https://github.com/knative-sandbox/reconciler-test/tree/master/pkg/test_images/eventshub
eventshub "toolkit" includes the code to perform assertions on the received events. All the mentioned features actually belong to eventshub I believe. Would you be game in exploring a port of the wathola image there?

I would love to unify our work, It allows us to reduce the overall amount of test images, which would be great. We have a lot of test code/test images with consequent tech debt we're trying to get rid of, and, as far as I see from the opened issues about wathola, I think we're aligned in the goals we're going for here and I'd love if we could take a more integrated approach towards the eventshub.

@cardil
Copy link
Contributor Author

cardil commented Feb 4, 2021

@slinkydeveloper I agree that we should unify those two in single configurable tooling.

But, I don't think, I should make this quite large unifying effort while addressing this issue.

Maybe even more, judging by features alone, it looks to me like wathola already have more features useful for upgrade tests then eventshub. That's why I'm not entirely convinced, that we should move wathola features to enventshub, and not the other way around.

@slinkydeveloper
Copy link
Contributor

But, I don't think, I should make this quite large unifying effort while addressing this issue.

What I'm saying is that we should address first the problem of having 2 test images with similar goals, and then move forward with addressing the remaining issues.

That's why I'm not entirely convinced, that we should move wathola features to enventshub, and not the other way around.

That's ok too for me, as long as we keep the "result image" in reconciler-test repo and it keeps the same features and the same assertion system (publish events via k8s events with EventInfo structure, all the various reply options, etc).

@cardil
Copy link
Contributor Author

cardil commented Feb 5, 2021

I have to do knative-extensions/eventing-kafka#67, so I really would like to do it here. And then make that refactoring unifying effort.

The other way around would make postponing knative-extensions/eventing-kafka#67 quite a lot, and that's not good.

@slinkydeveloper
Copy link
Contributor

Can we have this change to wathola "contained" only in eventing-kafka as a "temporary solution" and then we put our hands on the refactoring/unification effort? Sooner we unify that code, better is for everybody and simpler is to perform such unification. If we diverge too much in features, the unification task will just become more complex

@cardil
Copy link
Contributor Author

cardil commented Feb 5, 2021

@slinkydeveloper I can agree to such plan.

@github-actions
Copy link

github-actions bot commented May 7, 2021

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.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 7, 2021
cardil added a commit to cardil/knative-eventing that referenced this issue May 7, 2021
…ts (knative#4815)

Fixes knative#4812

Squashed commit of the following:

commit 603d63e
Author: Chris Suszyński <ksuszyns@redhat.com>
Date:   Mon Apr 19 14:23:20 2021 +0200

    Waiting until test port is open

commit 7c504fe
Merge: 1302587 97c0d30
Author: Chris Suszyński <ksuszyns@redhat.com>
Date:   Mon Apr 19 13:40:22 2021 +0200

    Merge remote-tracking branch 'upstream/main' into feature/wathola-sender-extendability

commit 1302587
Author: Chris Suszyński <ksuszyns@redhat.com>
Date:   Mon Apr 19 13:39:39 2021 +0200

    Allow of customization of wathola test images on downstream

commit 104175b
Author: Chris Suszyński <ksuszyns@redhat.com>
Date:   Mon Apr 19 13:37:37 2021 +0200

    Testing sender services

commit 50466ca
Merge: 225b5cf fc115ae
Author: Chris Suszyński <ksuszyns@redhat.com>
Date:   Thu Apr 15 13:48:13 2021 +0200

    Merge remote-tracking branch 'upstream/main' into feature/wathola-sender-extendability

commit 225b5cf
Author: Chris Suszyński <ksuszyns@redhat.com>
Date:   Fri Apr 9 14:00:41 2021 +0200

    Updating boilerplate

commit 4231946
Merge: 43b0773 4a3216c
Author: Chris Suszyński <ksuszyns@redhat.com>
Date:   Fri Apr 9 13:24:27 2021 +0200

    Merge remote-tracking branch 'upstream/main' into feature/wathola-sender-extendability

    Conflicts fixed:

     * test/upgrade/prober/wathola/sender/services.go

commit 43b0773
Author: Chris Suszyński <ksuszyns@redhat.com>
Date:   Mon Mar 29 21:22:48 2021 +0200

    Basic extendability of wathola sender

commit 530c00c
Merge: 38dcbc9 f882ff0
Author: Chris Suszyński <ksuszyns@redhat.com>
Date:   Fri Jan 29 21:08:18 2021 +0100

    Merge remote-tracking branch 'upstream/master' into feature/wathola-sender-extendability

    Conflicts fixed:
     * test/upgrade/prober/wathola/sender/services.go

commit 38dcbc9
Author: Chris Suszyński <ksuszyns@redhat.com>
Date:   Fri Jan 29 21:03:31 2021 +0100

    Clean ups

commit 1186af5
Author: Chris Suszyński <ksuszyns@redhat.com>
Date:   Fri Jan 29 20:47:34 2021 +0100

    EventSender interface to enable customisation of sending events
cardil added a commit to cardil/knative-eventing that referenced this issue May 19, 2021
…ts (knative#4815)

Fixes knative#4812

- Extension point for transport used to send events in upgrade tests
cardil added a commit to cardil/knative-eventing that referenced this issue May 19, 2021
…ts (knative#4815)

Fixes knative#4812

- Extension point for transport used to send events in upgrade tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/test-and-release Test infrastructure, tests or release kind/feature-request lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants