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

feat: add service discovery for kong admin service #3421

Merged
merged 4 commits into from
Feb 9, 2023

Conversation

pmalek
Copy link
Member

@pmalek pmalek commented Jan 20, 2023

What this PR does / why we need it:

This PR introduces kong Admin API service discovery via the --kong-admin-svc flag (to be discussed).

When that flag is specified (as namespace/service_name) KIC will watch EndpointSlices for that Service and add (and remove when the endpoint gets removed) a kong client for each endpoint for that Service.

This in its current form does not take into account the fact that gateway should be made ready only when its config is applied and ready (Admin API readines != Gateway Proxy readiness). We don't yet have a mechanism to be sure that a config has been applied (it can come in one of the upcoming Gateway releases) but we could implement a mechanism to make Gateway Proxy service marked as ready after the config has been sent.

#3499 covers whether we'd like to introduce said mechanism.

Which issue this PR fixes:

Fixes #3400

Special notes for your reviewer:

This is not yet 100% ready but ready for review.

The multi gw manifests that are also slightly changed in this PR are almost working, that is: they deploy KIC 2.8.1 which doesn't yet support --kong-admin-svc flag and since --kong-admin-url has a default of []string{"http://localhost:8001"} then it uses that but since with those manifests we deploy 2 separate deployments this address cannot be reached and KIC fails.

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

@pmalek pmalek added area/feature New feature or request wip labels Jan 20, 2023
@pmalek pmalek self-assigned this Jan 20, 2023
@pmalek pmalek force-pushed the single-controller-deployments-srv branch 7 times, most recently from de84bf3 to 6b69ed1 Compare January 26, 2023 20:05
@codecov
Copy link

codecov bot commented Jan 26, 2023

Codecov Report

Base: 72.9% // Head: 72.3% // Decreases project coverage by -0.6% ⚠️

Coverage data is based on head (9a400cf) compared to base (f6611e3).
Patch coverage: 48.3% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #3421     +/-   ##
=======================================
- Coverage   72.9%   72.3%   -0.6%     
=======================================
  Files        119     120      +1     
  Lines      14099   14318    +219     
=======================================
+ Hits       10283   10362     +79     
- Misses      3158    3289    +131     
- Partials     658     667      +9     
Impacted Files Coverage Δ
...ntrollers/configuration/kongadminapi_controller.go 0.0% <0.0%> (ø)
internal/dataplane/synchronizer.go 90.3% <0.0%> (-3.4%) ⬇️
internal/manager/utils/kongconfig/root.go 49.5% <0.0%> (ø)
internal/manager/config_validation.go 86.7% <25.0%> (-3.9%) ⬇️
internal/manager/setup.go 47.6% <40.0%> (-3.3%) ⬇️
internal/adminapi/client.go 56.0% <50.0%> (+4.3%) ⬆️
internal/adminapi/kong.go 61.7% <72.7%> (+0.6%) ⬆️
internal/dataplane/kong_client.go 85.1% <94.3%> (+1.6%) ⬆️
internal/manager/config.go 94.3% <100.0%> (+0.1%) ⬆️
internal/manager/controllerdef.go 98.7% <100.0%> (+<0.1%) ⬆️
... and 6 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@pmalek pmalek force-pushed the single-controller-deployments-srv branch 3 times, most recently from b37f9d7 to 3cb4067 Compare February 9, 2023 09:45
@pmalek
Copy link
Member Author

pmalek commented Feb 9, 2023

Here's an e2e targeted pipeline that passed: https://github.com/Kong/kubernetes-ingress-controller/actions/runs/4128494573/jobs/7133003972#step:10:374

test/e2e/all_in_one_test.go Outdated Show resolved Hide resolved
test/e2e/utils_test.go Show resolved Hide resolved
test/e2e/utils_test.go Outdated Show resolved Hide resolved
@pmalek pmalek mentioned this pull request Feb 9, 2023
6 tasks
@pmalek pmalek force-pushed the single-controller-deployments-srv branch 2 times, most recently from 1a694c1 to d808e16 Compare February 9, 2023 12:33
@pmalek pmalek requested a review from czeslavo February 9, 2023 12:34
czeslavo
czeslavo previously approved these changes Feb 9, 2023
@czeslavo
Copy link
Contributor

czeslavo commented Feb 9, 2023

🚢

@pmalek
Copy link
Member Author

pmalek commented Feb 9, 2023

@pmalek pmalek force-pushed the single-controller-deployments-srv branch from 9dbc558 to 9a400cf Compare February 9, 2023 13:58
@pmalek pmalek requested a review from czeslavo February 9, 2023 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/feature New feature or request size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Single controller deployments: Admin API endpoints tracking via EndpointSlice controller
3 participants