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

fix: disable Kong(Cluster)Plugin Programmed status #4584

Merged
merged 2 commits into from
Aug 29, 2023
Merged

Conversation

rainest
Copy link
Contributor

@rainest rainest commented Aug 29, 2023

What this PR does / why we need it:

Disables Kong(Cluster)Plugin status updates. Add TODOs for the relevant bug.

Which issue this PR fixes:

Multiple KIC instances in the same cluster will fight over plugin statuses because plugins have no class. We don't want them to have class, unless they're global.

Special notes for your reviewer:

Double duty as a release PR for #4582. AFAIK we're no longer doing updates other than the fixes themselves since we're now using relative tags, so the manifests remain unchanged.

A proper fix for this probably requires breaking out the plugin controller, so I think we want a hotfix for now. The feature isn't critical.

See https://github.com/Kong/kubernetes-ingress-controller/pull/4412/files for the original change. There are some other struct changes there, but AFAIK disabling the reconciler bit alone avoids the issue.

Didn't want to send this to main and backport since it touches generated code. I think we need to cherry-pick the first commit and generate separately if we don't have a more complete fix in for 2.12. There haven't been changes to the generated controllers since 2.11.0, so this one's actually fine.

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 Separately to main only. chore(doc) add 2.11.1 changelog #4583

Disable the Kong(Cluster)Plugin programmed condition. This caused a bug
in multi-class environments.

Add a TODO explaining the issue.
@rainest rainest added ci/run-e2e Trigger e2e test run from PR backport release/2.11.x labels Aug 29, 2023
@rainest rainest requested a review from a team as a code owner August 29, 2023 00:41
@team-k8s-bot
Copy link
Collaborator

E2E (targeted) tests with KIND-based clusters were started at https://github.com/Kong/kubernetes-ingress-controller/actions/runs/6006524843

@team-k8s-bot team-k8s-bot removed the ci/run-e2e Trigger e2e test run from PR label Aug 29, 2023
@team-k8s-bot
Copy link
Collaborator

E2E (targeted) tests with KIND-based clusters were started at https://github.com/Kong/kubernetes-ingress-controller/actions/runs/6006527185

@rainest rainest changed the title Fix/kongplugin status Disable Kong(Cluster)Plugin Programmed status and release 2.11.1 Aug 29, 2023
@codecov
Copy link

codecov bot commented Aug 29, 2023

Codecov Report

Patch coverage: 100.0% and project coverage change: -0.3% ⚠️

Comparison is base (1efcefd) 68.3% compared to head (e44bcf6) 68.1%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #4584     +/-   ##
=======================================
- Coverage   68.3%   68.1%   -0.3%     
=======================================
  Files        162     162             
  Lines      19030   18992     -38     
=======================================
- Hits       13014   12945     -69     
- Misses      5245    5291     +46     
+ Partials     771     756     -15     
Files Changed Coverage Δ
...trollers/configuration/zz_generated_controllers.go 33.6% <ø> (-5.2%) ⬇️
internal/manager/controllerdef.go 98.8% <100.0%> (+<0.1%) ⬆️

... and 8 files with indirect coverage changes

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

randmonkey
randmonkey previously approved these changes Aug 29, 2023
Copy link
Contributor

@randmonkey randmonkey left a comment

Choose a reason for hiding this comment

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

Do we want to include the CHANGELOG entry in this PR?

pmalek
pmalek previously approved these changes Aug 29, 2023
@pmalek pmalek enabled auto-merge (squash) August 29, 2023 08:22
@pmalek pmalek changed the title Disable Kong(Cluster)Plugin Programmed status and release 2.11.1 fix: disable Kong(Cluster)Plugin Programmed status Aug 29, 2023
@pmalek pmalek disabled auto-merge August 29, 2023 08:23
@pmalek pmalek enabled auto-merge (squash) August 29, 2023 08:23
@pmalek pmalek merged commit 2bafc2d into main Aug 29, 2023
269 of 279 checks passed
@pmalek pmalek deleted the fix/kongplugin-status branch August 29, 2023 09:00
github-actions bot pushed a commit that referenced this pull request Aug 29, 2023
* fix(crds) disable KongPlugin programmed condition

Disable the Kong(Cluster)Plugin programmed condition. This caused a bug
in multi-class environments.

Add a TODO explaining the issue.

* chore(*) make generate

* chore(doc): add changelog entry for 2.11.1

---------

Co-authored-by: Patryk Małek <patryk.malek@konghq.com>
(cherry picked from commit 2bafc2d)
pmalek added a commit that referenced this pull request Aug 29, 2023
… status (#4585)

* fix: disable Kong(Cluster)Plugin Programmed status (#4584)

* fix(crds) disable KongPlugin programmed condition

Disable the Kong(Cluster)Plugin programmed condition. This caused a bug
in multi-class environments.

Add a TODO explaining the issue.

* chore(*) make generate

* chore(doc): add changelog entry for 2.11.1

---------

Co-authored-by: Patryk Małek <patryk.malek@konghq.com>
(cherry picked from commit 2bafc2d)

* chore: trigger CI

---------

Co-authored-by: Travis Raines <571832+rainest@users.noreply.github.com>
Co-authored-by: Patryk Małek <patryk.malek@konghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants