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

Enable KafkaSource controller #1339

Conversation

pierDipi
Copy link
Member

@pierDipi pierDipi commented Oct 18, 2021

Fixes #312

Proposed Changes

  • Add E2E tests
  • Enable KafkaSource controller (webhook and controller handlers)
  • Add KafkaSource manifests

Release Note

None

Docs

None

@knative-prow-robot knative-prow-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. area/control-plane area/data-plane area/test approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 18, 2021
@google-cla google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Oct 18, 2021
@pierDipi
Copy link
Member Author

/hold

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 18, 2021
@codecov
Copy link

codecov bot commented Oct 18, 2021

Codecov Report

Merging #1339 (4fa9b21) into main (e54534f) will decrease coverage by 0.23%.
The diff coverage is 51.85%.

❗ Current head 4fa9b21 differs from pull request most recent head 0010c4d. Consider uploading reports for the commit 0010c4d to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1339      +/-   ##
============================================
- Coverage     76.43%   76.19%   -0.24%     
+ Complexity      584      583       -1     
============================================
  Files            98       98              
  Lines          3632     3659      +27     
  Branches        166      166              
============================================
+ Hits           2776     2788      +12     
- Misses          643      656      +13     
- Partials        213      215       +2     
Flag Coverage Δ
java-unittests 82.18% <ø> (-0.11%) ⬇️

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

Impacted Files Coverage Δ
control-plane/pkg/reconciler/source/controller.go 72.34% <50.00%> (-27.66%) ⬇️
control-plane/pkg/reconciler/source/source.go 58.01% <100.00%> (+0.32%) ⬆️
...dispatcher/impl/consumer/BaseConsumerVerticle.java 86.20% <0.00%> (-6.90%) ⬇️

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 e54534f...0010c4d. Read the comment docs.

@pierDipi pierDipi force-pushed the KNATIVE-312_Enable-KafkaSource-controller branch 2 times, most recently from 939b961 to 5e5edda Compare October 18, 2021 15:28
@pierDipi
Copy link
Member Author

/test pull-knative-sandbox-eventing-kafka-broker-integration-tests

@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 19, 2021
@pierDipi pierDipi force-pushed the KNATIVE-312_Enable-KafkaSource-controller branch from 0a56246 to a27adde Compare October 19, 2021 11:17
@knative-prow-robot knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 19, 2021
@pierDipi pierDipi force-pushed the KNATIVE-312_Enable-KafkaSource-controller branch 2 times, most recently from 83ef424 to 9583121 Compare October 25, 2021 08:13
@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 25, 2021
@pierDipi pierDipi force-pushed the KNATIVE-312_Enable-KafkaSource-controller branch 2 times, most recently from e2a19f5 to d317205 Compare October 26, 2021 08:39
@knative-prow-robot knative-prow-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 26, 2021
@pierDipi pierDipi force-pushed the KNATIVE-312_Enable-KafkaSource-controller branch from 1262fe5 to 721b545 Compare October 26, 2021 11:17
@knative-prow-robot knative-prow-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 26, 2021
@pierDipi pierDipi force-pushed the KNATIVE-312_Enable-KafkaSource-controller branch 3 times, most recently from 5c30a36 to 8efb3af Compare October 27, 2021 10:27
@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 2, 2021
@pierDipi pierDipi force-pushed the KNATIVE-312_Enable-KafkaSource-controller branch from f2bb169 to 2630a87 Compare November 3, 2021 09:09
@knative-prow-robot knative-prow-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 3, 2021
@pierDipi pierDipi force-pushed the KNATIVE-312_Enable-KafkaSource-controller branch from 2630a87 to 4ccaa7a Compare November 3, 2021 11:07
@knative-prow-robot knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 3, 2021
Signed-off-by: Pierangelo Di Pilato <pdipilat@redhat.com>
Signed-off-by: Pierangelo Di Pilato <pdipilat@redhat.com>
Signed-off-by: Pierangelo Di Pilato <pdipilat@redhat.com>
Signed-off-by: Pierangelo Di Pilato <pdipilat@redhat.com>
Signed-off-by: Pierangelo Di Pilato <pdipilat@redhat.com>
@pierDipi pierDipi force-pushed the KNATIVE-312_Enable-KafkaSource-controller branch from 4ccaa7a to abf2890 Compare November 3, 2021 12:59
@pierDipi pierDipi changed the title [WIP] Enable KafkaSource controller Enable KafkaSource controller Nov 3, 2021
@knative-prow-robot 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 Nov 3, 2021
@pierDipi pierDipi requested review from aliok and matzew November 3, 2021 15:01
@pierDipi
Copy link
Member Author

pierDipi commented Nov 3, 2021

/unhold

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 3, 2021
@pierDipi pierDipi force-pushed the KNATIVE-312_Enable-KafkaSource-controller branch from 4fa9b21 to a97991b Compare November 4, 2021 08:35
Signed-off-by: Pierangelo Di Pilato <pdipilat@redhat.com>
@pierDipi pierDipi force-pushed the KNATIVE-312_Enable-KafkaSource-controller branch from a97991b to 0010c4d Compare November 4, 2021 08:40
@knative-metrics-robot
Copy link

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/reconciler/source/controller.go 100.0% 80.0% -20.0

@aliok
Copy link
Member

aliok commented Nov 4, 2021

/lgtm
/approve

Thanks, looks great! We now have a KafkaSource impl!

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 4, 2021
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aliok, 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:

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

@knative-prow-robot knative-prow-robot merged commit 2a62bc0 into knative-extensions:main Nov 4, 2021
@pierDipi
Copy link
Member Author

pierDipi commented Nov 4, 2021

Thanks for your reviews!

@pierDipi pierDipi deleted the KNATIVE-312_Enable-KafkaSource-controller branch November 4, 2021 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/control-plane area/data-plane area/test cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement KafkaSource controller
5 participants