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.

Sign up for GitHub

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

Jump to bottom

Add KafkaChannel v1 API #4222

Merged
merged 5 commits into from
Jan 24, 2025
Merged

Add KafkaChannel v1 API #4222

merged 5 commits into from
Jan 24, 2025

Conversation

pierDipi
Copy link
Member

@pierDipi pierDipi commented Jan 21, 2025 •
edited
Loading

Part of #3956

Proposed Changes

  • Add KafkaChannel v1 types
  • Add conversions functions
  • Update CRD resource
  • Add conversion webhook

Release Note

Add KafkaChannel v1 API. The v1 API is backward compatible with the v1beta1 API.

Docs

@knative-prow knative-prow bot added area/control-plane size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 21, 2025
@knative-prow knative-prow bot requested review from Cali0707 and matzew January 21, 2025 15:38
@codecov Codecov
Copy link

codecov bot commented Jan 21, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0.81206% with 855 lines in your changes missing coverage. Please review.

Project coverage is 45.32%. Comparing base (b4d2bc2) to head (8b13c09).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...reconciler/messaging/v1/kafkachannel/reconciler.go 0.00% 245 Missing ⚠️
.../pkg/apis/messaging/v1/kafka_channel_validation.go 0.00% 103 Missing ⚠️
...reconciler/messaging/v1/kafkachannel/controller.go 0.00% 94 Missing ⚠️
...ioned/typed/messaging/v1/fake/fake_kafkachannel.go 0.00% 77 Missing ⚠️
...e/pkg/apis/messaging/v1/kafka_channel_lifecycle.go 0.00% 52 Missing ⚠️
...t/versioned/typed/messaging/v1/messaging_client.go 0.00% 44 Missing ⚠️
...tion/reconciler/messaging/v1/kafkachannel/state.go 0.00% 40 Missing ⚠️
...apis/messaging/v1beta1/kafka_channel_conversion.go 0.00% 32 Missing ⚠️
...ne/pkg/apis/messaging/v1/kafka_channel_defaults.go 0.00% 25 Missing ⚠️
...messaging/v1/kafkachannel/filtered/kafkachannel.go 0.00% 24 Missing ⚠️
... and 16 more
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #4222      +/-   ##
==========================================
- Coverage   47.33%   45.32%   -2.01%     
==========================================
  Files         270      290      +20     
  Lines       19011    19869     +858     
==========================================
+ Hits         8998     9005       +7     
- Misses       9300    10151     +851     
  Partials      713      713

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pierDipi pierDipi changed the title Add KafkaChannel v1 API [WIP] Add KafkaChannel v1 API Jan 22, 2025
@knative-prow knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 22, 2025
@pierDipi pierDipi changed the title [WIP] Add KafkaChannel v1 API Add KafkaChannel v1 API Jan 22, 2025
@knative-prow knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 22, 2025
matzew
matzew reviewed Jan 22, 2025
View reviewed changes
control-plane/pkg/apis/messaging/v1/kafka_channel_conversion.go
@@ -0,0 +1,34 @@
/*
Copyright 2020 The Knative Authors
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some linters are barking here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, however, those boilerplate comments are copies of the existing ones so I'll keep them as they are since that error will go away once we merge

@pierDipi
Copy link
Member Author

/test channel-integration-tests-sasl-plain

matzew
matzew approved these changes Jan 22, 2025
View reviewed changes
Copy link
Contributor

@matzew matzew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jan 22, 2025
@pierDipi
Copy link
Member Author

/retest

@matzew
Copy link
Contributor

matzew commented Jan 23, 2025

/test reconciler-tests

@matzew
Copy link
Contributor

matzew commented Jan 23, 2025

/go-in
/retest
/ship-it

@pierDipi
Copy link
Member Author

build-tests errors seems a real problem after that last commit merged on, need to re-run codegen

@pierDipi
KafkaChannel v1 API
41c10e7 
Signed-off-by: Pierangelo Di Pilato <[email protected]>
@pierDipi
Update codegen
5aba709 
Signed-off-by: Pierangelo Di Pilato <[email protected]>
@pierDipi
v1beta1 remains the stored version
d78b8ea 
Signed-off-by: Pierangelo Di Pilato <[email protected]>
@pierDipi
Fix service name in CRD
6c061c4 
Signed-off-by: Pierangelo Di Pilato <[email protected]>
@matzew
Copy link
Contributor

matzew commented Jan 23, 2025

/retest

@pierDipi
Update codegen
8b13c09 
Signed-off-by: Pierangelo Di Pilato <[email protected]>
@knative-prow knative-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2025
matzew
matzew approved these changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@matzew matzew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/retest

/lgtm

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jan 24, 2025
@knative-prow Knative Prow
Copy link

knative-prow bot commented Jan 24, 2025

[APPROVALNOTIFIER] This PR is APPROVED

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

@pierDipi
Copy link
Member Author

/retest

@knative-prow knative-prow bot merged commit ae8a755 into knative-extensions:main Jan 24, 2025
31 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matzew matzew matzew approved these changes

@Cali0707 Cali0707 Awaiting requested review from Cali0707

Assignees

@matzew matzew

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/control-plane 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pierDipi @matzew