Add Consumer reconciler for scheduling and scaling #1601

pierDipi · 2021-12-14T11:14:15Z

Part of #1537

Proposed Changes

Add Consumer reconciler for scheduling and scaling
Add ConsumerSpec validation based on the reconciler assumptions

Release Note

None

Docs

None

Note:

TODO: reconciler tests

codecov · 2021-12-14T11:19:09Z

Codecov Report

Merging #1601 (64bbe13) into main (6e55cf4) will decrease coverage by 6.54%.
The diff coverage is 75.00%.

@@             Coverage Diff              @@
##               main    #1601      +/-   ##
============================================
- Coverage     72.63%   66.09%   -6.55%     
- Complexity      600      606       +6     
============================================
  Files           122      128       +6     
  Lines          4586     5220     +634     
  Branches        174      176       +2     
============================================
+ Hits           3331     3450     +119     
- Misses          979     1484     +505     
- Partials        276      286      +10

Flag	Coverage Δ
java-unittests	`81.88% <ø> (+0.07%)`	⬆️

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

Impacted Files	Coverage Δ
...nals/kafka/eventing/v1alpha1/consumer_lifecycle.go	`0.00% <0.00%> (ø)`
...nternals/kafka/eventing/v1alpha1/consumer_types.go	`0.00% <ø> (ø)`
...als/kafka/eventing/v1alpha1/consumer_validation.go	`87.23% <82.35%> (-12.77%)`	⬇️
...fka/eventing/v1alpha1/consumer_group_validation.go	`100.00% <100.00%> (ø)`
control-plane/pkg/contract/extensions.go	`100.00% <100.00%> (ø)`
control-plane/pkg/contract/serde.go	`100.00% <100.00%> (ø)`
control-plane/pkg/reconciler/base/reconciler.go	`67.22% <100.00%> (ø)`
...r/dispatcher/main/ConsumerVerticleFactoryImpl.java	`87.82% <0.00%> (-0.25%)`	⬇️
...a/broker/dispatcher/impl/KafkaResponseHandler.java
...r/dispatcher/impl/ResponseToKafkaTopicHandler.java	`100.00% <0.00%> (ø)`
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e55cf4...64bbe13. Read the comment docs.

control-plane/pkg/contract/serde.go

control-plane/pkg/apis/internals/kafka/eventing/v1alpha1/consumer_types.go

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

pierDipi · 2022-01-10T07:53:33Z

I had to rebase this PR.

aavarghese · 2022-01-10T23:06:43Z

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

+	return idx
+}
+
+func (r Reconciler) schedule(ctx context.Context, logger *zap.Logger, c *kafkainternals.Consumer, mutatorFunc contractMutatorFunc) (bool, error) {


Can you explain what the schedule function does in terms of figuring out placements here? At this point, the consumer is already scheduled right? Maybe some more comments here might help as in what is the update to the config map being done, etc?

I'll add comment, maybe schedule as the name isn't the best one.

Does this help?

// schedule mutates the ConfigMap associated with the pod specified by Consumer.Spec.PodBind. // // The actual mutation is done by calling the provided contractMutatorFunc.

It does. Great. Think the function being called schedule made it a bit confusing without a comment

Signed-off-by: Pierangelo Di Pilato <pdipilat@redhat.com>

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

knative-metrics-robot · 2022-01-11T15:25:59Z

The following is the coverage report on the affected files.
Say /test pull-knative-sandbox-eventing-kafka-broker-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
control-plane/pkg/apis/internals/kafka/eventing/v1alpha1/consumer_validation.go	100.0%	84.8%	-15.2
control-plane/pkg/contract/extensions.go	Do not exist	100.0%
control-plane/pkg/contract/serde.go	Do not exist	100.0%

pierDipi · 2022-01-11T16:18:32Z

/retest

aavarghese

/lgtm

knative-prow-robot · 2022-01-12T15:47:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aavarghese, 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~~ [pierDipi]

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

pierDipi force-pushed the SRVKE-1060_Consumer-reconciler branch from 0f05994 to fed104c Compare December 14, 2021 13:49

knative-prow-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 14, 2021

pierDipi force-pushed the SRVKE-1060_Consumer-reconciler branch 5 times, most recently from b24bf38 to b2d6ea5 Compare December 14, 2021 14:44

pierDipi changed the title ~~[WIP] Add Consumer reconciler for scheduling and scaling~~ Add Consumer reconciler for scheduling and scaling Dec 14, 2021

knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 14, 2021

pierDipi requested review from matzew, aavarghese, aliok and devguyio December 14, 2021 15:11

matzew reviewed Dec 16, 2021

View reviewed changes

control-plane/pkg/contract/serde.go Show resolved Hide resolved

matzew reviewed Dec 16, 2021

View reviewed changes

control-plane/pkg/apis/internals/kafka/eventing/v1alpha1/consumer_types.go Outdated Show resolved Hide resolved

matzew reviewed Dec 16, 2021

View reviewed changes

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

pierDipi requested a review from matzew January 4, 2022 09:30

pierDipi force-pushed the SRVKE-1060_Consumer-reconciler branch 2 times, most recently from 12a6fd3 to 0e9a8be Compare January 10, 2022 07:52

devguyio requested review from devguyio and removed request for devguyio January 10, 2022 09:22

devguyio self-assigned this Jan 10, 2022

aavarghese reviewed Jan 10, 2022

View reviewed changes

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

pierDipi added 2 commits January 11, 2022 15:40

Add Consumer reconciler for scheduling and scaling

3d3de31

Signed-off-by: Pierangelo Di Pilato <pdipilat@redhat.com>

Refactor get* methods, rename + flatten structure

9693e7f

Signed-off-by: Pierangelo Di Pilato <pdipilat@redhat.com>

pierDipi force-pushed the SRVKE-1060_Consumer-reconciler branch from 0e9a8be to 9693e7f Compare January 11, 2022 15:03

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

pierDipi added 2 commits January 11, 2022 16:07

Add comment on schedule

ac0d313

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

Fix compilation error

64bbe13

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

aavarghese approved these changes Jan 12, 2022

View reviewed changes

knative-prow-robot assigned aavarghese Jan 12, 2022

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 12, 2022

knative-prow-robot merged commit 57e833c into knative-extensions:main Jan 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Consumer reconciler for scheduling and scaling #1601

Add Consumer reconciler for scheduling and scaling #1601

pierDipi commented Dec 14, 2021 •

edited

Loading

codecov bot commented Dec 14, 2021 •

edited

Loading

pierDipi commented Jan 10, 2022

aavarghese Jan 10, 2022

pierDipi Jan 11, 2022

pierDipi Jan 12, 2022

aavarghese Jan 12, 2022

knative-metrics-robot commented Jan 11, 2022

pierDipi commented Jan 11, 2022

aavarghese left a comment

knative-prow-robot commented Jan 12, 2022

Add Consumer reconciler for scheduling and scaling #1601

Add Consumer reconciler for scheduling and scaling #1601

Conversation

pierDipi commented Dec 14, 2021 • edited Loading

Proposed Changes

codecov bot commented Dec 14, 2021 • edited Loading

Codecov Report

pierDipi commented Jan 10, 2022

aavarghese Jan 10, 2022

Choose a reason for hiding this comment

pierDipi Jan 11, 2022

Choose a reason for hiding this comment

pierDipi Jan 12, 2022

Choose a reason for hiding this comment

aavarghese Jan 12, 2022

Choose a reason for hiding this comment

knative-metrics-robot commented Jan 11, 2022

pierDipi commented Jan 11, 2022

aavarghese left a comment

Choose a reason for hiding this comment

knative-prow-robot commented Jan 12, 2022

pierDipi commented Dec 14, 2021 •

edited

Loading

codecov bot commented Dec 14, 2021 •

edited

Loading