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: fix Kong client's status check #6689

Merged
merged 1 commit into from
Nov 19, 2024
Merged

Conversation

pmalek
Copy link
Member

@pmalek pmalek commented Nov 18, 2024

What this PR does / why we need it:

This PR fixes an invalid status check in kong client which causes the client to report that it's ready while in reality it's not. The problem arises from an invalid check in NewKongClientForWorkspace()

	if wsName == "" {
		return NewClient(client), nil
	}

which short circuits when the workspace is empty and does not perform the checks that follow.

The above causes the client manager to think that this client is ready to receive config where it's not.

This behavior can easily be tested with using Gateway Discovery and scaling up the Kong Deployment. This also prevents the KongConfigurationApplyFailed event from being created.

2024-11-18T16:57:50+01:00	info	controllers.KongAdminAPIService	Reconciling Admin API EndpointSlice	{"v": 0, "namespace": "kong", "name": "kong-kong-admin-skgmz"}
2024-11-18T16:57:50+01:00	debug	controllers.KongAdminAPIService	Notifying about newly detected Admin APIs	{"v": 1, "admin_apis": ["https://10.244.0.27:8444", "https://10.244.0.185:8444"]}
2024-11-18T16:57:50+01:00	debug	Received notification about Admin API addresses change	{"v": 1}
2024-11-18T16:57:50+01:00	debug	setup.readiness-checker	Checked readiness of pending client	{"address": "https://10.244.0.185:8444", "v": 1, "ok": true}
2024-11-18T16:57:50+01:00	debug	controllers.EndpointSlice	Reconciling resource	{"DiscoveryV1EndpointSlice": {"name":"kong-kong-proxy-rsfsw","namespace":"kong"}, "v": 1, "namespace": "kong", "name": "kong-kong-proxy-rsfsw"}
2024-11-18T16:57:50+01:00	debug	setup.readiness-checker	Already created client is ready	{"address": "https://10.244.0.27:8444", "v": 1}
2024-11-18T16:57:50+01:00	debug	Notifying subscribers about gateway clients change	{"v": 1}
2024-11-18T16:57:51+01:00	debug	controllers.KongLicense	Get license from KongLicense resource	{"v": 1, "name": "kong-license"}
2024-11-18T16:57:51+01:00	debug	New configuration snapshot detected	{"v": 1, "hash": "L4QTCQCY47VAEMJYAZSJTGEF23XIL23BKP3PN5DDX4ZJKDYGDPEQ===="}
2024-11-18T16:57:51+01:00	debug	Parsing kubernetes objects into data-plane configuration	{"v": 1}
2024-11-18T16:57:51+01:00	debug	controllers.KongLicense	Get license from KongLicense resource	{"v": 1, "name": "kong-license"}
2024-11-18T16:57:51+01:00	debug	Successfully built data-plane configuration	{"v": 1, "duration": "392.667µs"}
2024-11-18T16:57:51+01:00	debug	Sending configuration to gateway clients	{"v": 1, "urls": ["https://10.244.0.27:8444", "https://10.244.0.185:8444"]}
2024-11-18T16:57:51+01:00	debug	Shipping config to diagnostic server	{"url": "https://10.244.0.185:8444", "v": 1}
2024-11-18T16:57:51+01:00	debug	events	failed to apply Kong configuration to https://10.244.0.185:8444: config update failed: failed to reload declarative configuration: failed posting new config to /config: making HTTP request: Post "https://10.244.0.185:8444/config?check_hash=1&flatten_errors=1": dial tcp 10.244.0.185:8444: connect: connection refused	{"v": 1, "type": "Warning", "object": {"kind":"Pod","namespace":"kong","name":"local","apiVersion":"v1"}, "reason": "KongConfigurationApplyFailed"}
2024-11-18T16:57:51+01:00	debug	No configuration change, skipping sync to Kong	{"url": "https://10.244.0.27:8444", "v": 1}
2024-11-18T16:57:51+01:00	debug	Shipping config to diagnostic server	{"url": "https://10.244.0.27:8444", "v": 1}
2024-11-18T16:57:51+01:00	debug	Skipping recovery from gateways sync error - not enough details to recover	{"v": 1, "error": "performing update for https://10.244.0.185:8444 failed: config update failed: failed to reload declarative configuration: failed posting new config to /config: making HTTP request: Post \"https://10.244.0.185:8444/config?check_hash=1&flatten_errors=1\": dial tcp 10.244.0.185:8444: connect: connection refused"}
2024-11-18T16:57:51+01:00	error	dataplane-synchronizer	Could not update kong admin	{"error": "performing update for https://10.244.0.185:8444 failed: config update failed: failed to reload declarative configuration: failed posting new config to /config: making HTTP request: Post \"https://10.244.0.185:8444/config?check_hash=1&flatten_errors=1\": dial tcp 10.244.0.185:8444: connect: connection refused"}
2024-11-18T16:57:51+01:00	debug	events	successfully applied Kong configuration to https://10.244.0.27:8444	{"v": 1, "type": "Normal", "object": {"kind":"Pod","namespace":"kong","name":"local","apiVersion":"v1"}, "reason": "KongConfigurationSucceeded"}
2024-11-18T16:57:52+01:00	debug	controllers.EndpointSlice	Reconciling resource	{"DiscoveryV1EndpointSlice": {"name":"kong-kong-proxy-rsfsw","namespace":"kong"}, "v": 1, "namespace": "kong", "name": "kong-kong-proxy-rsfsw"}
2024-11-18T16:57:52+01:00	debug	controllers.EndpointSlice	Reconciling resource	{"DiscoveryV1EndpointSlice": {"name":"kong-kong-manager-zw2v2","namespace":"kong"}, "v": 1, "namespace": "kong", "name": "kong-kong-manager-zw2v2"}
2024-11-18T16:57:52+01:00	debug	controllers.EndpointSlice	Reconciling resource	{"DiscoveryV1EndpointSlice": {"name":"kong-kong-admin-skgmz","namespace":"kong"}, "v": 1, "namespace": "kong", "name": "kong-kong-admin-skgmz"}
2024-11-18T16:57:52+01:00	info	controllers.KongAdminAPIService	Reconciling Admin API EndpointSlice	{"v": 0, "namespace": "kong", "name": "kong-kong-admin-skgmz"}
2024-11-18T16:57:54+01:00	debug	controllers.KongLicense	Get license from KongLicense resource	{"v": 1, "name": "kong-license"}
2024-11-18T16:57:54+01:00	debug	New configuration snapshot detected	{"v": 1, "hash": "CEE2CSUNBAWL5TVBC7UGGKH4RFOEATG7WGRW6CCC7UEU5VN4TJGA===="}
2024-11-18T16:57:54+01:00	debug	Parsing kubernetes objects into data-plane configuration	{"v": 1}
2024-11-18T16:57:54+01:00	debug	controllers.KongLicense	Get license from KongLicense resource	{"v": 1, "name": "kong-license"}
2024-11-18T16:57:54+01:00	debug	Successfully built data-plane configuration	{"v": 1, "duration": "70.292µs"}
2024-11-18T16:57:54+01:00	debug	Sending configuration to gateway clients	{"v": 1, "urls": ["https://10.244.0.27:8444", "https://10.244.0.185:8444"]}
2024-11-18T16:57:54+01:00	debug	No configuration change, skipping sync to Kong	{"url": "https://10.244.0.27:8444", "v": 1}
2024-11-18T16:57:54+01:00	debug	Shipping config to diagnostic server	{"url": "https://10.244.0.27:8444", "v": 1}
2024-11-18T16:57:54+01:00	debug	events	successfully applied Kong configuration to https://10.244.0.27:8444	{"v": 1, "type": "Normal", "object": {"kind":"Pod","namespace":"kong","name":"local","apiVersion":"v1"}, "reason": "KongConfigurationSucceeded"}
2024-11-18T16:57:54+01:00	info	Successfully synced configuration to Kong	{"url": "https://10.244.0.185:8444", "update_strategy": "InMemory", "v": 0, "duration": "202ms"}
2024-11-18T16:57:54+01:00	debug	Shipping config to diagnostic server	{"url": "https://10.244.0.185:8444", "v": 1}
2024-11-18T16:57:54+01:00	debug	Triggering report for configured Kubernetes objects	{"v": 1, "count": 0}
2024-11-18T16:57:54+01:00	debug	events	successfully applied Kong configuration to https://10.244.0.185:8444	{"v": 1, "type": "Normal", "object": {"kind":"Pod","namespace":"kong","name":"local","apiVersion":"v1"}, "reason": "KongConfigurationSucceeded"}

Which issue this PR fixes:

Special notes for your reviewer:

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 this to the KIC v3.4.x milestone Nov 18, 2024
@pmalek pmalek self-assigned this Nov 18, 2024
@pmalek pmalek force-pushed the fix-kong-client-status-check branch from 93aadef to d05938a Compare November 18, 2024 16:06
Copy link

codecov bot commented Nov 18, 2024

Codecov Report

Attention: Patch coverage is 65.21739% with 8 lines in your changes missing coverage. Please review.

Project coverage is 77.9%. Comparing base (61a1f60) to head (1c455d7).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
internal/adminapi/kong.go 60.0% 5 Missing and 3 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #6689     +/-   ##
=======================================
+ Coverage   77.7%   77.9%   +0.1%     
=======================================
  Files        203     204      +1     
  Lines      23836   23903     +67     
=======================================
+ Hits       18534   18631     +97     
+ Misses      4351    4324     -27     
+ Partials     951     948      -3     

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


🚨 Try these New Features:

@pmalek pmalek force-pushed the fix-kong-client-status-check branch from d05938a to 0b289e5 Compare November 18, 2024 17:31
@pull-request-size pull-request-size bot added size/L and removed size/M labels Nov 18, 2024
@pmalek pmalek force-pushed the fix-kong-client-status-check branch 2 times, most recently from cf9b8f7 to 1d2421a Compare November 18, 2024 17:51
@pmalek pmalek marked this pull request as ready for review November 18, 2024 17:58
@pmalek pmalek requested a review from a team as a code owner November 18, 2024 17:58
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.

Good catch 🎖️

@pmalek pmalek requested a review from czeslavo November 19, 2024 13:03
@pmalek pmalek force-pushed the fix-kong-client-status-check branch from 1d2421a to cef96bc Compare November 19, 2024 13:03
@pmalek pmalek force-pushed the fix-kong-client-status-check branch from cef96bc to 1c455d7 Compare November 19, 2024 13:17
@pmalek pmalek merged commit c451ed3 into main Nov 19, 2024
42 checks passed
@pmalek pmalek deleted the fix-kong-client-status-check branch November 19, 2024 13:41
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.

2 participants