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

GH-545 restrict default geo to local #549

Merged
merged 1 commit into from
Apr 25, 2024
Merged

GH-545 restrict default geo to local #549

merged 1 commit into from
Apr 25, 2024

Conversation

maksymvavilov
Copy link
Contributor

@maksymvavilov maksymvavilov commented Apr 16, 2024

Prevent creation of the default endpoint if we are using the LoadBalanced strategy and the lb-attribute-geo-code label on a gateway does not match the default geo in the policy spec

@maksymvavilov maksymvavilov linked an issue Apr 16, 2024 that may be closed by this pull request
Copy link

codecov bot commented Apr 16, 2024

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 31.97%. Comparing base (ece13e8) to head (50c36ef).
Report is 52 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #549       +/-   ##
===========================================
- Coverage   80.20%   31.97%   -48.24%     
===========================================
  Files          64       65        +1     
  Lines        4492     4910      +418     
===========================================
- Hits         3603     1570     -2033     
- Misses        600     3247     +2647     
+ Partials      289       93      -196     
Flag Coverage Δ
integration ?
unit 31.97% <0.00%> (+1.94%) ⬆️

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

Components Coverage Δ
api/v1beta1 (u) ∅ <ø> (∅)
api/v1beta2 (u) 47.43% <53.33%> (-44.00%) ⬇️
pkg/common (u) 77.47% <ø> (-11.35%) ⬇️
pkg/istio (u) 44.37% <ø> (-29.54%) ⬇️
pkg/log (u) 36.84% <ø> (-57.90%) ⬇️
pkg/reconcilers (u) ∅ <ø> (∅)
pkg/rlptools (u) 57.25% <ø> (-22.21%) ⬇️
controllers (i) 7.23% <0.00%> (-69.57%) ⬇️
Files Coverage Δ
controllers/dns_helper.go 0.00% <0.00%> (-83.98%) ⬇️

... and 48 files with indirect coverage changes

@maksymvavilov maksymvavilov changed the base branch from main to distributed-dns April 16, 2024 13:17
@maksymvavilov maksymvavilov changed the base branch from distributed-dns to main April 16, 2024 13:17
@maksymvavilov maksymvavilov changed the base branch from main to distributed-dns April 16, 2024 13:22
@maksymvavilov maksymvavilov marked this pull request as ready for review April 16, 2024 22:05
@maksymvavilov maksymvavilov requested a review from a team as a code owner April 16, 2024 22:05
@philbrookes philbrookes force-pushed the distributed-dns branch 3 times, most recently from eb6acdf to b8d0863 Compare April 17, 2024 07:45
@maksymvavilov maksymvavilov changed the title [WIP] GH-545 restrict default geo to local GH-545 restrict default geo to local Apr 17, 2024
@maleck13 maleck13 force-pushed the distributed-dns branch 4 times, most recently from 7ad8e81 to 6032133 Compare April 22, 2024 07:54
@maksymvavilov maksymvavilov changed the base branch from distributed-dns to main April 25, 2024 10:29
Copy link
Member

@mikenairn mikenairn left a comment

Choose a reason for hiding this comment

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

Changes look good, and along with #578, should resolve the default geo issue.

Unfortunately the tests are failing, but looks like an unrelated flake.

Lets get this merged first and I'll deal with updating mine.

@mikenairn
Copy link
Member

Build images job failing for odd reason, created an issue for it #580

@mikenairn
Copy link
Member

Verified locally that the DNSPolicy tests are still working:

./bin/ginkgo -p --procs 10 --focus "DNSPolicy" -tags integration ./controllers/...
Running Suite: Controller Suite - /home/mnairn/go/src/github.com/kuadrant/kuadrant-operator/controllers
=======================================================================================================
Random Seed: 1714062964

Will run 20 of 147 specs
Running in parallel across 10 processes
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS•••••••••SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS•••••••••••

Ran 20 of 147 Specs in 43.668 seconds
SUCCESS! -- 20 Passed | 0 Failed | 0 Pending | 127 Skipped


Ginkgo ran 1 suite in 50.270835805s
Test Suite Passed

Don't like merging with failing tests but needs must.

@mikenairn mikenairn merged commit 45e157b into main Apr 25, 2024
17 of 21 checks passed
@mikenairn mikenairn deleted the GH-545 branch April 25, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Update DNSPolicy default geo logic
2 participants