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

KRT for GG #10193

Open
wants to merge 110 commits into
base: main
Choose a base branch
from
Open

KRT for GG #10193

wants to merge 110 commits into from

Conversation

lgadban
Copy link
Contributor

@lgadban lgadban commented Oct 9, 2024

Description

First drop of bringing KRT to GG.

In this PR, we accomplish the following high-level items:

  • Use a separate component to handle the translation and syncing of Kube Gateway Proxies
  • Use KRT to generate the ApiSnapshot used in Proxy->xDS translation

Caveats:
As we only are taking on the ApiSnapshot generation in this pass, the "boundary" of the KRT system is fairly narrow.
This means that we still have controller-runtime driving proxy resyncs (in addition to any of the work done by KRT's dependency tracking)

Status for RouteOptions and VHostOptions used in more than 1 proxy does not work, see https://github.com/solo-io/solo-projects/issues/7044

Unit testing
The currently proxy_syncer component (and the xds_syncer that preceded it) had no actual unit tests due to the nature of the work it is doing.
This PR does not add any unit tests but we will address this in a follow up, see: https://github.com/solo-io/solo-projects/issues/7056

Garbage collection
We need to implement garbage collection for kube gw Proxies

API changes

Code changes

CI changes

Docs changes

Context

Interesting decisions

Testing steps

Notes for reviewers

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

stevenctl and others added 30 commits August 15, 2024 09:22
pluginRegistry := s.k8sGwExtensions.CreatePluginRegistry(ctx)
rm := reports.NewReportMap()
r := reports.NewReporter(&rm)
podClient := kclient.New[*corev1.Pod](s.istioClient)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
podClient := kclient.New[*corev1.Pod](s.istioClient)
podClient := kclient.NewFiltered[*corev1.Pod](s.istioClient, kclient.Filter{
ObjectTransform: kube.StripPodUnusedFields,
})

This gets rid of ManagedFields which is large and almost definitely unused

if err != nil {
// This should never happen, try again?
return
kubeUpstreams := setupCollectionDynamic[glookubev1.Upstream](
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: the name to me indicates Upstream_Kube when it means the k8s resource. Would just upstreams be ok? Or rawUpstreams maybe?

upstreamSpecs,
)
if len(warns) > 0 || len(errs) > 0 {
// FIXME
Copy link
Contributor

Choose a reason for hiding this comment

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

the old code just logged in this case

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lgadban
Copy link
Contributor Author

lgadban commented Oct 16, 2024

/kick-ci

@lgadban
Copy link
Contributor Author

lgadban commented Oct 16, 2024

/kick

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work in progress signals bulldozer to keep pr open (don't auto-merge)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants