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

chore: update router default #4934

Merged
merged 11 commits into from
Oct 27, 2023
Merged

chore: update router default #4934

merged 11 commits into from
Oct 27, 2023

Conversation

rainest
Copy link
Contributor

@rainest rainest commented Oct 20, 2023

What this PR does / why we need it:

Changes the router default to expressions.

Which issue this PR fixes:

Part of #4719

Special notes for your reviewer:

See #3693 (comment) for areas of the code that interact with this setting but were not updated, usually because they handle any allowed router value.

Per other commentary in that issue, we had an open question on whether to use expressions or traditional_compatible. It looked like we were proceeding with expressions elsewhere, so I went with that. Opening this PR to sorta prod that decision forward.

Either way, we update the same locations, so if we do want to use traditional_compatible instead, it's easy enough to update the diff.

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

@rainest rainest added the do not merge let the author merge this, don't merge for them. label Oct 20, 2023
@rainest rainest marked this pull request as ready for review October 20, 2023 23:34
@rainest rainest requested a review from a team as a code owner October 20, 2023 23:34
@codecov
Copy link

codecov bot commented Oct 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1351004) 77.6% compared to head (d04bd85) 75.3%.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #4934     +/-   ##
=======================================
- Coverage   77.6%   75.3%   -2.3%     
=======================================
  Files        167     167             
  Lines      18709   18709             
=======================================
- Hits       14527   14102    -425     
- Misses      3345    3782    +437     
+ Partials     837     825     -12     

see 16 files with indirect coverage changes

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

@rainest rainest marked this pull request as draft October 23, 2023 18:48
@rainest
Copy link
Contributor Author

rainest commented Oct 23, 2023

Manifest updates as written wouldn't work since they use the same envvar across DB-less and DB-backed. I need to figure out how to either replace that (complicated by the {name: whatever, value: whatever} structure of envvars not providing a path or take the lazy approach and just move it out of the root base.

@rainest rainest added the ci/run-e2e Trigger e2e test run from PR label Oct 24, 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/6628345628

@team-k8s-bot team-k8s-bot removed the ci/run-e2e Trigger e2e test run from PR label Oct 24, 2023
@rainest rainest marked this pull request as ready for review October 24, 2023 14:31
@rainest
Copy link
Contributor Author

rainest commented Oct 24, 2023

Alright, with the guidance to have DB-less default to expressions and DB-backed to traditional_compatible, revised kustomize bits, and a revised matrix, I think this is now ready.

config/variants/postgres/kustomization.yaml Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
.github/workflows/_integration_tests.yaml Show resolved Hide resolved
@rainest rainest removed the do not merge let the author merge this, don't merge for them. label Oct 25, 2023
@rainest rainest requested a review from pmalek October 25, 2023 11:10
@pmalek pmalek added this to the KIC v3.0.0 milestone Oct 25, 2023
pmalek
pmalek previously approved these changes Oct 25, 2023
@pmalek pmalek added the ci/run-e2e Trigger e2e test run from PR label Oct 26, 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/6654763042

@randmonkey randmonkey merged commit a25cef5 into main Oct 27, 2023
35 checks passed
@randmonkey randmonkey deleted the chore/router-default branch October 27, 2023 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants