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

define KongLicense CRD #5487

Merged
merged 7 commits into from
Jan 30, 2024
Merged

define KongLicense CRD #5487

merged 7 commits into from
Jan 30, 2024

Conversation

randmonkey
Copy link
Contributor

@randmonkey randmonkey commented Jan 25, 2024

What this PR does / why we need it:

define KongLicense CRD for storing a Kong license and applying it to Kong gateway.

Which issue this PR fixes:

First step of #5437

Special notes for your reviewer:

Not finished yet, but the structure of the CRD is basically determined. Major uncertain parts:

  • How to name the status of the KongLicense reconciled in each controller, parents or controllers? (controllers)
  • Should we import types from gateway API, or define our own types? (define own types)
  • Do we need a field to store the type of the controller? (defined a controllerName to identify controllers)
  • Should we have Phase and what phases do we have? Pending -> Accepted -> Configured? (No Phase, but conditions)
  • Should we add conditions and transition time between each step? (Ditto above)

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@randmonkey randmonkey marked this pull request as ready for review January 25, 2024 10:33
@randmonkey randmonkey requested a review from a team as a code owner January 25, 2024 10:33
@randmonkey randmonkey added do not merge let the author merge this, don't merge for them. work in progress Work In Progress area/CRD Changes in existing CRDs or introduction of new ones labels Jan 25, 2024
Copy link
Contributor

@czeslavo czeslavo left a comment

Choose a reason for hiding this comment

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

How to name the status of the KongLicense reconciled in each controller, parents or controllers?

I think that controllers is more precise, but I'm OK with parents as well.

Should we import types from gateway API, or define our own types?

I think we should not rely on Gateway API in this case as it's not a CRD that's somehow interconnected with this "world".

Do we need a field to store the type of the controller?

I think we don't as long as we keep the ControllerName unique between controllers/controllers' instances (as I wrote in the comment).

Should we have Phase and what phases do we have? Pending -> Accepted -> Configured?

I'd skip Phase for now and just use []metav1.Conditions which is more common and easy to extend.

Should we add conditions and transition time between each step?

Ditto.

pkg/apis/configuration/v1alpha1/kong_license_types.go Outdated Show resolved Hide resolved
pkg/apis/configuration/v1alpha1/kong_license_types.go Outdated Show resolved Hide resolved
pkg/apis/configuration/v1alpha1/kong_license_types.go Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jan 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f5ae15f) 69.7% compared to head (a8c9a7e) 69.8%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #5487   +/-   ##
=====================================
  Coverage   69.7%   69.8%           
=====================================
  Files        176     176           
  Lines      22857   22873   +16     
=====================================
+ Hits       15949   15982   +33     
+ Misses      5961    5948   -13     
+ Partials     947     943    -4     

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

@randmonkey randmonkey changed the title [WIP] define KongLicense CRD define KongLicense CRD Jan 26, 2024
@randmonkey randmonkey removed do not merge let the author merge this, don't merge for them. work in progress Work In Progress labels Jan 26, 2024
@randmonkey randmonkey force-pushed the define_kong_license_crd branch 3 times, most recently from b92b8cd to ca1bc85 Compare January 30, 2024 07:23
programmer04
programmer04 previously approved these changes Jan 30, 2024
czeslavo
czeslavo previously approved these changes Jan 30, 2024
programmer04
programmer04 previously approved these changes Jan 30, 2024
@czeslavo czeslavo dismissed stale reviews from programmer04 and themself via 2f465a1 January 30, 2024 11:04
czeslavo
czeslavo previously approved these changes Jan 30, 2024
programmer04
programmer04 previously approved these changes Jan 30, 2024
Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
@tao12345666333
Copy link
Member

fixed the lint error.

@randmonkey randmonkey merged commit 9af08c2 into main Jan 30, 2024
37 checks passed
@randmonkey randmonkey deleted the define_kong_license_crd branch January 30, 2024 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CRD Changes in existing CRDs or introduction of new ones size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants