Pass eventtype owner to data plane #3284

Cali0707 · 2023-08-10T19:08:51Z

Part of #3181

We need the eventtype Reference/Owner information in the dataplane so that we can auto create event types in #3181.

Proposed Changes

Pass the event type ownership info through the data plane contract

…tocreate Signed-off-by: Calum Murray <cmurray@redhat.com>

Cali0707 · 2023-08-10T19:09:03Z

/cc @Leo6Leo @matzew @creydr

codecov · 2023-08-10T19:15:43Z

Codecov Report

Merging #3284 (96f709f) into main (5b46ea2) will decrease coverage by 0.04%.
Report is 4 commits behind head on main.
The diff coverage is 64.70%.

❗ Current head 96f709f differs from pull request most recent head 02c3426. Consider uploading reports for the commit 02c3426 to get more accurate results

@@             Coverage Diff              @@
##               main    #3284      +/-   ##
============================================
- Coverage     61.54%   61.50%   -0.04%     
- Complexity      761      764       +3     
============================================
  Files           181      181              
  Lines         12316    12338      +22     
  Branches        265      267       +2     
============================================
+ Hits           7580     7589       +9     
- Misses         4140     4144       +4     
- Partials        596      605       +9

Flag	Coverage Δ
java-unittests	`70.88% <50.00%> (+0.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
control-plane/pkg/reconciler/broker/broker.go	`72.65% <100.00%> (+0.10%)`	⬆️
control-plane/pkg/reconciler/channel/channel.go	`69.21% <100.00%> (+0.10%)`	⬆️
...ntrol-plane/pkg/reconciler/channel/v2/channelv2.go	`71.06% <100.00%> (+0.09%)`	⬆️
control-plane/pkg/reconciler/sink/kafka_sink.go	`75.38% <100.00%> (+0.15%)`	⬆️
.../kafka/broker/core/eventtype/EventTypeBuilder.java	`92.00% <100.00%> (+0.69%)`	⬆️
.../core/reconciler/impl/ResourcesReconcilerImpl.java	`85.44% <0.00%> (-1.10%)`	⬇️
control-plane/pkg/contract/contract.pb.go	`7.34% <0.00%> (-0.07%)`	⬇️

... and 3 files with indirect coverage changes

Cali0707 · 2023-08-10T20:13:03Z

/retest-required

Cali0707 · 2023-08-11T17:35:47Z

/retest-required

creydr · 2023-08-17T14:26:26Z

/test upgrade-tests

Leo6Leo · 2023-08-17T15:10:58Z

/test upgrade-tests

Cali0707 · 2023-08-17T15:58:09Z

I think the upgrade test failures might be real failures, I will look into it

pierDipi · 2023-08-21T09:39:33Z

control-plane/pkg/reconciler/broker/broker.go

+			EventTypeOwnerReference: &contract.Reference{
+				Uuid:      string(broker.UID),
+				Name:      broker.Name,
+				Namespace: broker.Namespace,
+				Kind:      brokerKind,
+				Version:   brokerAPIVersion,
+			},


Why not including the missing fields in reference field?

@pierDipi is there a different reference field I could be including them on?

Yes, it's called Reference under Resource

eventing-kafka-broker/proto/contract.proto

Lines 333 to 339 in 0242a87

// Resource reference.

//

// This reference is used to reference the associated resource for data plane

// activities such as:

// - setting the `source` attribute of a KafkaSource event (when it's not a CloudEvent)

// - tagging metrics

Reference reference = 11;

Ah okay I was only looking at the ingress object, my bad! I'll update this PR

control-plane/pkg/reconciler/broker/broker.go

…ocreate Signed-off-by: Calum Murray <cmurray@redhat.com>

Signed-off-by: Calum Murray <cmurray@redhat.com>

Cali0707 · 2023-08-22T20:10:44Z

/cc @pierDipi @Leo6Leo

Signed-off-by: Calum Murray <cmurray@redhat.com>

Leo6Leo · 2023-08-28T14:11:51Z

/lgtm

Leo6Leo · 2023-08-28T14:12:55Z

/retest-required

Cali0707 · 2023-08-28T15:22:10Z

@Leo6Leo I think the test failure is an actual issue, not a flaky failure here. Not sure what is causing the issue though...

pierDipi · 2023-09-06T15:07:46Z

control-plane/pkg/reconciler/testing/objects_channel.go

@@ -259,6 +259,8 @@ func ChannelReference() *contract.Reference {
 		Uuid:      ChannelUUID,
 		Namespace: ChannelNamespace,
 		Name:      ChannelName,
+		Kind:      "KafkaChannel",
+		Version:   messagingv1beta1.SchemeGroupVersion.String(),


I think Version here is meant to be ResourceVersion as opposed to GroupVersion

Oh okay, I need the group version so I will add a new field for that then. Thanks!

Signed-off-by: Calum Murray <cmurray@redhat.com>

Cali0707 · 2023-09-20T17:26:10Z

Now that changes to the contract are backwards compatible, the tests are passing on this PR.

/cc @pierDipi @Leo6Leo

Signed-off-by: Calum Murray <cmurray@redhat.com>

Cali0707 · 2023-09-25T15:15:28Z

/test upgrade-tests

pierDipi · 2023-10-02T13:47:03Z

/retest-required

Cali0707 · 2023-10-04T17:10:48Z

/retest-required

Cali0707 · 2023-10-06T12:38:42Z

/test upgrade-tests

Signed-off-by: Calum Murray <cmurray@redhat.com>

Cali0707 · 2023-11-20T14:32:44Z

/retest

pierDipi

/lgtm
/approve

knative-prow · 2023-11-20T17:16:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Cali0707, pierDipi

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~~ [Cali0707,pierDipi]

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

Cali0707 · 2023-11-20T18:28:49Z

/test reconciler-tests

knative-prow · 2023-11-20T19:35:59Z

@Cali0707: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
reconciler-tests-keda_eventing-kafka-broker_main	`3c85417`	link	false	`/test reconciler-tests-keda`
reconciler-tests-loom_eventing-kafka-broker_main	`3c85417`	link	false	`/test reconciler-tests-loom`
reconciler-tests-namespaced-broker-loom_eventing-kafka-broker_main	`3c85417`	link	false	`/test reconciler-tests-namespaced-broker-loom`

Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Updated contract to pass necessary info to dataplane for eventtype au…

498f45c

…tocreate Signed-off-by: Calum Murray <cmurray@redhat.com>

knative-prow bot requested review from creydr, Leo6Leo, matzew and odacremolbap August 10, 2023 19:11

knative-prow bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. area/control-plane area/data-plane labels Aug 10, 2023

pierDipi reviewed Aug 21, 2023

View reviewed changes

control-plane/pkg/reconciler/broker/broker.go Outdated Show resolved Hide resolved

Updated contract reference to have required fields for event type aut…

09f2c23

…ocreate Signed-off-by: Calum Murray <cmurray@redhat.com>

Cali0707 force-pushed the pass-eventtype-owner-to-data-plane branch from 280b408 to 09f2c23 Compare August 22, 2023 20:07

knative-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 22, 2023

Fixed imports

e1b3774

Signed-off-by: Calum Murray <cmurray@redhat.com>

knative-prow bot requested a review from pierDipi August 22, 2023 20:10

Reconcile contract reference properly for channel v2

9a47f59

Signed-off-by: Calum Murray <cmurray@redhat.com>

knative-prow bot assigned Leo6Leo Aug 28, 2023

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Aug 28, 2023

pierDipi reviewed Sep 6, 2023

View reviewed changes

Updated to use GroupVersion instead of ResourceVersion

f756a17

Signed-off-by: Calum Murray <cmurray@redhat.com>

knative-prow bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 6, 2023

knative-prow bot requested a review from pierDipi September 20, 2023 17:26

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 23, 2023

Fix merge conflicts

3c85417

Signed-off-by: Calum Murray <cmurray@redhat.com>

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 25, 2023

Cali0707 added 3 commits October 6, 2023 10:42

Merge branch 'main' into pass-eventtype-owner-to-data-plane

bd6a3b7

maybe it is the resource equality checks

96f709f

Signed-off-by: Calum Murray <cmurray@redhat.com>

Don't use the new contract fields

1965336

Signed-off-by: Calum Murray <cmurray@redhat.com>

knative-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 11, 2023

remove unused imports

02c3426

Signed-off-by: Calum Murray <cmurray@redhat.com>

pierDipi approved these changes Nov 20, 2023

View reviewed changes

knative-prow bot assigned pierDipi Nov 20, 2023

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Nov 20, 2023

knative-prow bot merged commit ee8d67d into knative-extensions:main Nov 20, 2023
31 of 34 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass eventtype owner to data plane #3284

Pass eventtype owner to data plane #3284

Cali0707 commented Aug 10, 2023

Cali0707 commented Aug 10, 2023

codecov bot commented Aug 10, 2023 •

edited

Loading

Cali0707 commented Aug 10, 2023

Cali0707 commented Aug 11, 2023

creydr commented Aug 17, 2023

Leo6Leo commented Aug 17, 2023

Cali0707 commented Aug 17, 2023

pierDipi Aug 21, 2023 •

edited

Loading

Cali0707 Aug 21, 2023

pierDipi Aug 22, 2023

Cali0707 Aug 22, 2023

Cali0707 commented Aug 22, 2023

Leo6Leo commented Aug 28, 2023

Leo6Leo commented Aug 28, 2023

Cali0707 commented Aug 28, 2023

pierDipi Sep 6, 2023

Cali0707 Sep 6, 2023

Cali0707 commented Sep 20, 2023

Cali0707 commented Sep 25, 2023

pierDipi commented Oct 2, 2023

Cali0707 commented Oct 4, 2023

Cali0707 commented Oct 6, 2023

Cali0707 commented Nov 20, 2023

pierDipi left a comment

knative-prow bot commented Nov 20, 2023

Cali0707 commented Nov 20, 2023

knative-prow bot commented Nov 20, 2023 •

edited

Loading

	// Resource reference.
	//
	// This reference is used to reference the associated resource for data plane
	// activities such as:
	// - setting the `source` attribute of a KafkaSource event (when it's not a CloudEvent)
	// - tagging metrics
	Reference reference = 11;

Pass eventtype owner to data plane #3284

Pass eventtype owner to data plane #3284

Conversation

Cali0707 commented Aug 10, 2023

Proposed Changes

Cali0707 commented Aug 10, 2023

codecov bot commented Aug 10, 2023 • edited Loading

Codecov Report

Cali0707 commented Aug 10, 2023

Cali0707 commented Aug 11, 2023

creydr commented Aug 17, 2023

Leo6Leo commented Aug 17, 2023

Cali0707 commented Aug 17, 2023

pierDipi Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

Cali0707 Aug 21, 2023

Choose a reason for hiding this comment

pierDipi Aug 22, 2023

Choose a reason for hiding this comment

Cali0707 Aug 22, 2023

Choose a reason for hiding this comment

Cali0707 commented Aug 22, 2023

Leo6Leo commented Aug 28, 2023

Leo6Leo commented Aug 28, 2023

Cali0707 commented Aug 28, 2023

pierDipi Sep 6, 2023

Choose a reason for hiding this comment

Cali0707 Sep 6, 2023

Choose a reason for hiding this comment

Cali0707 commented Sep 20, 2023

Cali0707 commented Sep 25, 2023

pierDipi commented Oct 2, 2023

Cali0707 commented Oct 4, 2023

Cali0707 commented Oct 6, 2023

Cali0707 commented Nov 20, 2023

pierDipi left a comment

Choose a reason for hiding this comment

knative-prow bot commented Nov 20, 2023

Cali0707 commented Nov 20, 2023

knative-prow bot commented Nov 20, 2023 • edited Loading

codecov bot commented Aug 10, 2023 •

edited

Loading

pierDipi Aug 21, 2023 •

edited

Loading

knative-prow bot commented Nov 20, 2023 •

edited

Loading