Skip to content

kpr: Enhance kube-proxy replacement logging #41133

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 27, 2025
Merged

kpr: Enhance kube-proxy replacement logging #41133

merged 1 commit into from
Aug 27, 2025

Conversation

liyihuang
Copy link
Contributor

@liyihuang liyihuang commented Aug 13, 2025 •
edited
Loading

improves logging for the kpr at before start up and outputs the configuration being used to follow the practice that we followed before here.
https://github.com/cilium/cilium/blob/v1.17/daemon/cmd/kube_proxy_replacement.go#L48-L52

Log kube-proxy replacement config before starting kube-proxy replacement

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Aug 13, 2025
@liyihuang liyihuang added area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. release-note/misc This PR makes changes that have no direct user impact. labels Aug 13, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Aug 13, 2025
@liyihuang
Copy link
Contributor Author

/test

@liyihuang liyihuang force-pushed the pr/liyihuang/kpr_start_log branch from 7ad6044 to e8ea365 Compare August 13, 2025 19:07
@liyihuang
Copy link
Contributor Author

/test

@liyihuang liyihuang force-pushed the pr/liyihuang/kpr_start_log branch 2 times, most recently from 2f7e3f0 to 7356503 Compare August 14, 2025 13:36
@liyihuang
Copy link
Contributor Author

/test

@liyihuang liyihuang force-pushed the pr/liyihuang/kpr_start_log branch from 7356503 to ff7ff19 Compare August 15, 2025 18:46
@liyihuang
Copy link
Contributor Author

/test

@liyihuang liyihuang force-pushed the pr/liyihuang/kpr_start_log branch from ff7ff19 to 9ca752b Compare August 16, 2025 01:39
@liyihuang
Copy link
Contributor Author

/test

@liyihuang liyihuang force-pushed the pr/liyihuang/kpr_start_log branch 3 times, most recently from 591e849 to 4b3e390 Compare August 19, 2025 19:13
@liyihuang
Copy link
Contributor Author

/test

@liyihuang liyihuang marked this pull request as ready for review August 19, 2025 21:59
@liyihuang liyihuang requested a review from a team as a code owner August 19, 2025 21:59
@liyihuang liyihuang requested a review from jrife August 19, 2025 21:59
jrife
jrife reviewed Aug 19, 2025
View reviewed changes
Copy link
Contributor

@jrife jrife left a comment

Choose a reason for hiding this comment

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

improves logging for the kpr at before start up and outputs the configuration being used to follow the practice that we followed before here.

It looks like it used to log this stuff but that got dropped at some point?

@liyihuang
Copy link
Contributor Author

improves logging for the kpr at before start up and outputs the configuration being used to follow the practice that we followed before here.

It looks like it used to log this stuff but that got dropped at some point?

yes. I guess because we are deprecating most of configs through configmap and might think we can just drop this log. However, I have found that we always need EnableSocketLB flag exposed for users and it's disable by default. Then it's very confusing from user perspective when they see EnableSocketLB is disabled when agent loads the config but KPR enables it quitely.

@liyihuang liyihuang force-pushed the pr/liyihuang/kpr_start_log branch from 4b3e390 to 5a889c4 Compare August 20, 2025 02:18
@liyihuang liyihuang requested a review from a team as a code owner August 20, 2025 02:18
@liyihuang liyihuang requested a review from thorn3r August 20, 2025 02:18
jrife
jrife approved these changes Aug 20, 2025
View reviewed changes
@liyihuang liyihuang force-pushed the pr/liyihuang/kpr_start_log branch 2 times, most recently from bbd9b2b to 0d86203 Compare August 20, 2025 03:42
@liyihuang
Copy link
Contributor Author

/test

@brb
Copy link
Member

brb commented Aug 20, 2025

Thanks! Could you change the target branch to v1.18?

@liyihuang
Copy link
Contributor Author

Thanks! Could you change the target branch to v1.18?

If I'm not mistaken, we also need this for main since we still can configure socketLB flag with kpr in 1.19. Shouldn't we merge to main backport to 1.18?

@brb brb added the needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch label Aug 21, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Aug 25, 2025
@jrife
Copy link
Contributor

jrife commented Aug 25, 2025

@liyihuang Just a heads up. It looks like you need to resolve a merge conflict in pkg/logging/logfields/logfields.go

@liyihuang liyihuang force-pushed the pr/liyihuang/kpr_start_log branch from 0d86203 to 9560b3c Compare August 25, 2025 20:22
@jrife
Copy link
Contributor

jrife commented Aug 25, 2025

/test

@liyihuang
kpr: Enhance kube-proxy replacement logging
f324f40 
improves logging for the kpr before start up and outputs the configuration
to follow the practice that we did before here.
https://github.com/cilium/cilium/blob/v1.17/daemon/cmd/kube_proxy_replacement.go#L48-L52

Signed-off-by: Liyi Huang <liyi.huang@isovalent.com>
@liyihuang liyihuang force-pushed the pr/liyihuang/kpr_start_log branch from 9560b3c to f324f40 Compare August 25, 2025 22:49
@liyihuang
Copy link
Contributor Author

/test

@joestringer joestringer added this pull request to the merge queue Aug 27, 2025
Merged via the queue into cilium:main with commit 8dce8f9 Aug 27, 2025
68 checks passed
@viktor-kurchenko viktor-kurchenko mentioned this pull request Sep 2, 2025
Merged
18 tasks
@viktor-kurchenko viktor-kurchenko added backport-pending/1.18 The backport for Cilium 1.18.x for this PR is in progress. and removed needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch labels Sep 2, 2025
@github-actions github-actions bot added backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. and removed backport-pending/1.18 The backport for Cilium 1.18.x for this PR is in progress. labels Sep 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brb brb brb approved these changes

@jrife jrife jrife approved these changes

@thorn3r thorn3r thorn3r approved these changes

Assignees
No one assigned
Labels
area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@liyihuang @brb @jrife @thorn3r @joestringer @viktor-kurchenko