Skip to content

Remove ServiceCache, ServiceManager and legacy LB types #40022

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 11 commits into from
Jun 16, 2025
Merged

Remove ServiceCache, ServiceManager and legacy LB types #40022

merged 11 commits into from
Jun 16, 2025

Conversation

joamaki
Copy link
Contributor

@joamaki joamaki commented Jun 13, 2025 •
edited
Loading

This is the final piece of the load-balancing control-plane cleanup to fully remove the old load-balancing control-plane implementation and types.

  • clustermesh/services_test.go tested same stuff as clustermesh/script_test.go. Extended the testdata/ to cover what was in services_test.go and deleted it rather than converting it from using ServiceCache (thanks @giorio94)
  • Removed ServiceCache, ServiceManager, LegacyBackend and LegacySVC
  • Dropped the "cilium-dbg service get" after discussing it over with @brb
  • Reimplemented the /service REST API without going through the legacy types.
  • Fixed the services feature metric
  • Dropped the CiliumEnvoyConfig resources as they were not used anymore
  • Implemented kube proxy healthz against BPFOps to return last service update time (Not entirely sure this is useful, should we just drop it and return latest time as lastUpdated?)
"cilium-dbg service get" has been removed. Use "cilium-dbg service list" or "cilium-dbg shell -- db/get frontends 1.2.3.4:80/TCP" instead.

@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 Jun 13, 2025
@joamaki joamaki added the release-note/misc This PR makes changes that have no direct user impact. label Jun 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 Jun 13, 2025
@joamaki joamaki force-pushed the pr/joamaki/lb-cleanup-2 branch 3 times, most recently from 490b507 to d71d3d2 Compare June 13, 2025 11:27
@joamaki
Copy link
Contributor Author

joamaki commented Jun 13, 2025

/test

@joamaki joamaki requested review from brb and giorio94 June 13, 2025 11:29
@joamaki joamaki marked this pull request as ready for review June 13, 2025 11:29
@joamaki joamaki requested review from a team as code owners June 13, 2025 11:29
@joamaki joamaki force-pushed the pr/joamaki/lb-cleanup-2 branch from d71d3d2 to fa4b1cc Compare June 13, 2025 12:34
@joamaki
Copy link
Contributor Author

joamaki commented Jun 13, 2025

/test

giorio94
giorio94 approved these changes Jun 13, 2025
View reviewed changes
Copy link
Member

@giorio94 giorio94 left a comment

Choose a reason for hiding this comment

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

/lgtm for my codeowners, thanks!

@brb brb mentioned this pull request Jun 13, 2025
Merged
@joamaki joamaki force-pushed the pr/joamaki/lb-cleanup-2 branch from fa4b1cc to 3738272 Compare June 13, 2025 14:02
@joamaki
Copy link
Contributor Author

joamaki commented Jun 13, 2025

/test

@joestringer joestringer added the release-blocker/1.18 This issue will prevent the release of the next version of Cilium. label Jun 13, 2025
@joestringer
Copy link
Member

I added the release-blocker label only for consistency given that #39767 and #40026 are proposed as blockers and they are both blocked on this. I am not advocating for an exception to feature freeze, but probably @cilium/sig-lb can weigh in on that.

joestringer
joestringer approved these changes Jun 13, 2025
View reviewed changes
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Changes LGTM for my code owners.

joamaki added 11 commits June 13, 2025 22:06
@joamaki
clustermesh: Remove services_test.go
b397e2a 
Extend testdata/ to cover all tests that services_test.go does and
remove services_test.go.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki
k8s: Remove ServiceCache and Service
0f9891e 
These are now unused.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki
loadbalancer: Simplify ServiceManager
7691169 
Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki
api,cilium-dbg: Drop "service get" support
27b496d 
The ID-based lookup of services has been deprecated since v1.14, so finally remove it.

We have now better ways of looking up services, e.g. via "db/get frontends" etc.
To find by ID one can use "service list" (maybe with -o json) or via "db/show frontends -f json"
and search with 'jq'.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki
loadbalancer: Reimplement REST API without legacy types
6ac3d46 
Implement conversion of [Frontend] to [models.Service] and
switch the /service and /debug handlers to querying against
the frontend table.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki
loadbalacer: Remove legacy Backend and SVC
4db247e 
These are no longer used.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki
hubble,loadbalancer: Remove ServiceManager.GetServiceNameByAddr
016dcdb 
Do a lookup from frontends table in the hubble payload parser and
drop the GetServiceNameByAddr.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki
healthz,loadbalancer: Implement LastUpdatedAt with BPFOps
7894035 
Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki
loadbalancer: Remove adapters and ServiceManager interface
87ef740 
Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki
daemon,k8s: Remove unused CiliumEnvoyConfig Resource ctors
908151b 
These constructors are no longer used. If a sub-system needs to access CECs
then it should be using Table[*ciliumenvoyconfig.CEC].

Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki
bugtool: Remove cilium_lb* events dump
1dbb6aa 
These maps no longer are configured with WithEvents(). Given that
we have good record of what was reconciled in Table[*Frontend] the
events buffer isn't that useful.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki joamaki force-pushed the pr/joamaki/lb-cleanup-2 branch from ab6b6eb to 1dbb6aa Compare June 13, 2025 20:07
@joamaki
Copy link
Contributor Author

joamaki commented Jun 13, 2025

/test

@qmonnet qmonnet removed their request for review June 13, 2025 23:12
@joamaki joamaki removed the request for review from tommyp1ckles June 16, 2025 09:19
@joamaki joamaki added this pull request to the merge queue Jun 16, 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 Jun 16, 2025
Merged via the queue into cilium:main with commit 47727d4 Jun 16, 2025
68 of 71 checks passed
@joamaki joamaki deleted the pr/joamaki/lb-cleanup-2 branch June 16, 2025 09:51
@github-project-automation github-project-automation bot moved this from Proposed to Done in Release blockers Jun 16, 2025
@giorio94 giorio94 mentioned this pull request Jun 18, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaworu kaworu kaworu approved these changes

@brb brb brb approved these changes

@squeed squeed squeed approved these changes

@joestringer joestringer joestringer approved these changes

@aanm aanm aanm approved these changes

@glrf glrf glrf approved these changes

@giorio94 giorio94 giorio94 approved these changes

Assignees
No one assigned
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-blocker/1.18 This issue will prevent the release of the next version of Cilium. release-note/misc This PR makes changes that have no direct user impact.
Projects
Release blockers
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@joamaki @joestringer @kaworu @brb @squeed @aanm @glrf @giorio94