Enable Knative Kafka w/o upstream e2e tests and w/o smoke tests #587

aliok · 2020-10-20T10:29:16Z

Enable controller and webhook
No smoke tests yet (they are in progress: https://github.com/lberk/serverless-operator/tree/knativekafkae2e)
No upstream e2e tests enabled yet (we have SRVKE-590 to enable the e2e test)
Serverless operator e2e tests (deployment shape check) is added
Use our images as image override is to be added later (for now we need our own images, due to patches - with the CSV override we go back to gcr.io for the upstream manifest)
Had to play with go mod because of WIP: Run kafka, next eventing and last serving #581 (review)

knative-operator/deploy/resources/knativekafka/kafkachannel-v0.17.1.yaml

openshift/ci-operator/source-image/Dockerfile

README.md

knative-operator/deploy/resources/knativekafka/kafkachannel-v0.17.1.yaml

markusthoemmes · 2020-10-20T13:40:07Z

olm-catalog/serverless-operator/manifests/operator_v1alpha1_knativekafka_crd.yaml

@@ -0,0 +1,116 @@
+apiVersion: apiextensions.k8s.io/v1beta1


Should this be the v1 based CRD?

+1 on v1 CRD.

I have moved my "old" PR for CRD_v1 now to Ali's brach:

aliok#5

See the old reference for the size of the DIFF - I've now closed my old CRD_v1 PR

Gonna merge that after CI reports success/fail here

Don't want to interrupt it. I want to see the results

Let's do this separately. I had trouble doing this. I received strange OLM errors. Perhaps unrelated.
Doing it in a separate PR makes more sense anyway since we already have the CRD with v1beta1 in master and that change is not in this PR's scope

Did my PR for your branch not work @aliok ?

But I can do it later as separate PR 🤷🏻‍♂️

Yeah, let's do that separately once we merge this PR.

markusthoemmes · 2020-10-20T13:40:49Z

templates/csv.yaml

+      - description: The version of Knative Kafka installed
+        displayName: Version
+        path: version


Have we added that? I recall a discussion where we did decide not to add it.

+1 good point. gotta remove that.

I removed this

test/e2e-tests.sh

test/v1alpha1/knativekafka.go

aliok · 2020-10-21T10:39:42Z

@markusthoemmes I did rebase and adressed the comments.

Couldn't try locally yet though because my crc instance keeps crashing. There might be some failures in the CI since I haven't verified anything locally but this should not block your review.

UPDATE: worked locally now. All good, ready for review

markusthoemmes

/lgtm
/approve
/hold

Holding as I'd like to see everything pass and want to doublecheck the logs to contain the new stuff as expected once green.

Great job! 🎉

aliok · 2020-10-21T11:28:59Z

Pushed another commit. Realized the env vars in the makefile were wrong.

aliok · 2020-10-21T12:09:30Z

/test 4.5-upgrade-tests-aws-ocp-45
/test 4.5-upstream-e2e-aws-ocp-45

error after 39m45s. (more info)
Job execution failed: Pod got deleted unexpectedly

Thanks @openshift-ci-robot !

aliok · 2020-10-21T12:38:40Z

/test 4.5-operator-e2e-aws-ocp-45

Just retesting one

aliok · 2020-10-21T12:46:38Z

Yes, one of the build clusters is borked. We're working on moving jobs to the other one.

Response I got from #forum-testplatform

aliok · 2020-10-21T13:01:33Z

/test 4.5-operator-e2e-aws-ocp-45

aliok · 2020-10-21T13:12:34Z

/test 4.5-upstream-e2e-aws-ocp-45
/test 4.5-upgrade-tests-aws-ocp-45

test/v1alpha1/knativekafka.go

markusthoemmes · 2020-10-21T16:57:30Z

KnativeKafka doesn't come up

{"level":"error","ts":"2020-10-21T14:13:15.586Z","logger":"kubebuilder.controller","msg":"Reconciler error","controller":"knativekafka-controller","request":"knative-eventing/knative-kafka","error":"failed to apply manifest: ServiceAccount \"kafka-ch-controller\" is invalid: [metadata.ownerReferences.apiVersion: Invalid value: \"\": version must not be empty, metadata.ownerReferences.kind: Invalid value: \"\": kind must not be empty]","stacktrace":"github.com/openshift-knative/serverless-operator/knative-operator/vendor/github.com/go-logr/zapr.(*zapLogger).Error\n\t/go/src/github.com/openshift-knative/serverless-operator/knative-operator/vendor/github.com/go-logr/zapr/zapr.go:128\ngithub.com/openshift-knative/serverless-operator/knative-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\t/go/src/github.com/openshift-knative/serverless-operator/knative-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:217\ngithub.com/openshift-knative/serverless-operator/knative-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func1\n\t/go/src/github.com/openshift-knative/serverless-operator/knative-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:158\ngithub.com/openshift-knative/serverless-operator/knative-operator/vendor/k8s.io/apimachinery/pkg/util/wait.JitterUntil.func1\n\t/go/src/github.com/openshift-knative/serverless-operator/knative-operator/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:133\ngithub.com/openshift-knative/serverless-operator/knative-operator/vendor/k8s.io/apimachinery/pkg/util/wait.JitterUntil\n\t/go/src/github.com/openshift-knative/serverless-operator/knative-operator/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:134\ngithub.com/openshift-knative/serverless-operator/knative-operator/vendor/k8s.io/apimachinery/pkg/util/wait.Until\n\t/go/src/github.com/openshift-knative/serverless-operator/knative-operator/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:88"}

test/v1alpha1/knativekafka.go

matzew · 2020-10-21T17:58:19Z

/test 4.5-operator-e2e-aws-ocp-45

Let;s see if issue persists...

aliok · 2020-10-21T20:20:52Z

Rebased, addressed the errors.Wrapf() comment, fixed the order of things in operator e2e tests

matzew · 2020-10-22T05:35:54Z

/lgtm

@markusthoemmes feel free to unhold, after Ali addressed the recent comment - thanks for reviews! ❤️

markusthoemmes

/lgtm
/approve

Abfahrt!

openshift-ci-robot · 2020-10-22T06:13:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aliok, markusthoemmes, matzew

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [aliok,markusthoemmes,matzew]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

markusthoemmes · 2020-10-22T06:19:44Z

/unhold

openshift-ci-robot requested review from devguyio and rhuss October 20, 2020 10:29

openshift-ci-robot added the approved label Oct 20, 2020

aliok changed the title ~~Enable Knative Kafka w/o upstream e2e tests and w/o smoke tests~~ [WIP] Enable Knative Kafka w/o upstream e2e tests and w/o smoke tests Oct 20, 2020

openshift-ci-robot added the do-not-merge/work-in-progress label Oct 20, 2020

matzew mentioned this pull request Oct 20, 2020

Introduce a new e2e target, that runs all (Serving/Eventing and event… #591

Merged

matzew force-pushed the 001-enable-knative-kafka branch from 534fea6 to 2927617 Compare October 20, 2020 13:02

matzew changed the title ~~[WIP] Enable Knative Kafka w/o upstream e2e tests and w/o smoke tests~~ Enable Knative Kafka w/o upstream e2e tests and w/o smoke tests Oct 20, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress label Oct 20, 2020

matzew reviewed Oct 20, 2020

View reviewed changes

knative-operator/deploy/resources/knativekafka/kafkachannel-v0.17.1.yaml Outdated Show resolved Hide resolved

markusthoemmes reviewed Oct 20, 2020

View reviewed changes

This was referenced Oct 20, 2020

Adding CRD v1 and additionalPrinterColumns aliok/serverless-operator#5

Closed

Add test leg test-e2e-with-kafka #599

Merged

aliok force-pushed the 001-enable-knative-kafka branch from 2927617 to 49a303f Compare October 21, 2020 10:34

markusthoemmes approved these changes Oct 21, 2020

View reviewed changes

openshift-ci-robot added the do-not-merge/hold label Oct 21, 2020

openshift-ci-robot assigned markusthoemmes Oct 21, 2020

openshift-ci-robot added lgtm and removed lgtm labels Oct 21, 2020

markusthoemmes reviewed Oct 21, 2020

View reviewed changes

test/v1alpha1/knativekafka.go Outdated Show resolved Hide resolved

markusthoemmes reviewed Oct 21, 2020

View reviewed changes

test/v1alpha1/knativekafka.go Outdated Show resolved Hide resolved

test/v1alpha1/knativekafka.go Outdated Show resolved Hide resolved

aliok added 5 commits October 21, 2020 23:15

Enable KnativeKafka

54d28ef

Remove version statusDescriptor for KnativeKafka

7f2fa40

Fix env vars in makefile

5ccf152

No errors.Wrapf(...)

5a1e976

Install serverless operator before downstream tests

14d7171

aliok force-pushed the 001-enable-knative-kafka branch from e33979e to 14d7171 Compare October 21, 2020 20:15

Fix a problem when creating KnativeKafka in operator e2e tests

78b1665

openshift-ci-robot assigned matzew Oct 22, 2020

openshift-ci-robot added the lgtm label Oct 22, 2020

markusthoemmes approved these changes Oct 22, 2020

View reviewed changes

openshift-ci-robot removed the do-not-merge/hold label Oct 22, 2020

openshift-merge-robot merged commit 24090bf into openshift-knative:master Oct 22, 2020

This was referenced Oct 22, 2020

Umbrella issue for having Kafka components #486

Closed

KnativeKafka switch to CRD_v1 and adding additionalPrinterColumns #607

Merged

aliok deleted the 001-enable-knative-kafka branch March 9, 2023 10:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable Knative Kafka w/o upstream e2e tests and w/o smoke tests #587

Enable Knative Kafka w/o upstream e2e tests and w/o smoke tests #587

aliok commented Oct 20, 2020 •

edited by matzew

Loading

markusthoemmes Oct 20, 2020

aliok Oct 20, 2020

matzew Oct 20, 2020

aliok Oct 20, 2020

aliok Oct 21, 2020

matzew Oct 21, 2020

aliok Oct 21, 2020

markusthoemmes Oct 20, 2020

aliok Oct 20, 2020

aliok Oct 21, 2020

aliok commented Oct 21, 2020 •

edited

Loading

markusthoemmes left a comment

aliok commented Oct 21, 2020

aliok commented Oct 21, 2020

aliok commented Oct 21, 2020

aliok commented Oct 21, 2020

aliok commented Oct 21, 2020

aliok commented Oct 21, 2020

markusthoemmes commented Oct 21, 2020

matzew commented Oct 21, 2020

aliok commented Oct 21, 2020

matzew commented Oct 22, 2020

markusthoemmes left a comment

openshift-ci-robot commented Oct 22, 2020

markusthoemmes commented Oct 22, 2020

Enable Knative Kafka w/o upstream e2e tests and w/o smoke tests #587

Enable Knative Kafka w/o upstream e2e tests and w/o smoke tests #587

Conversation

aliok commented Oct 20, 2020 • edited by matzew Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aliok commented Oct 21, 2020 • edited Loading

markusthoemmes left a comment

Choose a reason for hiding this comment

aliok commented Oct 21, 2020

aliok commented Oct 21, 2020

aliok commented Oct 21, 2020

aliok commented Oct 21, 2020

aliok commented Oct 21, 2020

aliok commented Oct 21, 2020

markusthoemmes commented Oct 21, 2020

matzew commented Oct 21, 2020

aliok commented Oct 21, 2020

matzew commented Oct 22, 2020

markusthoemmes left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Oct 22, 2020

markusthoemmes commented Oct 22, 2020

aliok commented Oct 20, 2020 •

edited by matzew

Loading

aliok commented Oct 21, 2020 •

edited

Loading