Don't use Continue option with cached client

[gjkim42]

What this PR does / why we need it:

This PR makes cache-backed clients not use Continue option to support more than 100 httpRoutes, as the option is not implemented in CacheClient. (It never returns Continue token.)

Which issue this PR fixes:

fixes #4456

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

[CLAassistant]

All committers have signed the CLA.

[pmalek]

Hi @gjkim42 👋

Do you have any links to references that would back this up 🤔?

If that's indeed broken (i.e. the paging is not working with manager's Client) we'd ideally prefer to see a test for this ( to verify the breakage itself and the fix ).

We perform very similar list at

kubernetes-ingress-controller/internal/adminapi/endpoints.go

Lines 75 to 96 in 08c67a9

	var endpointsList discoveryv1.EndpointSliceList
	if err := kubeClient.List(ctx, &endpointsList, &client.ListOptions{
	LabelSelector: labelSelector,
	Namespace: service.Namespace,
	Continue: continueToken,
	Limit: defaultEndpointSliceListPagingLimit,
	}); err != nil {
	return nil, err
	}
	for _, es := range endpointsList.Items {
	adminAPI, err := d.AdminAPIsFromEndpointSlice(es)
	if err != nil {
	return nil, err
	}
	addresses = addresses.Union(adminAPI)
	}
	if endpointsList.Continue == "" {
	break
	}
	continueToken = endpointsList.Continue

but that's done with client.New() at

kubernetes-ingress-controller/internal/manager/config.go

Line 328 in 0dfd102

return client.New(conf, client.Options{})

and not the manager's client that's created by the https://pkg.go.dev/sigs.k8s.io/controller-runtime#NewManager at

kubernetes-ingress-controller/internal/manager/run.go

Line 129 in 08c67a9

mgr, err := ctrl.NewManager(kubeconfig, controllerOpts)

🤔

[czeslavo]

I think that's more of a bug in the upstream. We're using client.Reader / client.Client interface here and I would expect every implementation (cached or not cached) to be compliant with the possible options it accepts in its List method. I wrote about it under the upstream issue: kubernetes-sigs/controller-runtime#2439

To not block ourselves with the upstream process of fixing that, I'm ok with the workaround implemented in this PR, but as it was mentioned, it'd be great to have this covered in an envtest before merging.

[gjkim42]

Do you have any links to references that would back this up 🤔?

I've investigated the code here, the CacheReader never sets the Continue token in its output.

We are having a PR related to this issue in the upstream repository here recently.

We perform very similar list at

kubernetes-ingress-controller/internal/adminapi/endpoints.go

Lines 75 to 96 in 08c67a9

	var endpointsList discoveryv1.EndpointSliceList
	if err := kubeClient.List(ctx, &endpointsList, &client.ListOptions{
	LabelSelector: labelSelector,
	Namespace: service.Namespace,
	Continue: continueToken,
	Limit: defaultEndpointSliceListPagingLimit,
	}); err != nil {
	return nil, err
	}
	for _, es := range endpointsList.Items {
	adminAPI, err := d.AdminAPIsFromEndpointSlice(es)
	if err != nil {
	return nil, err
	}
	addresses = addresses.Union(adminAPI)
	}
	if endpointsList.Continue == "" {
	break
	}
	continueToken = endpointsList.Continue

but that's done with client.New() at

Unlike this patch, I think that the code may work fine, as it is, if the client is not CacheReader.

---

it'd be great to have this covered in an envtest before merging.

I'll add one.

[gjkim42]

• added TestHTTPRouteGetAttachedRoutesMoreThan100 integration test
• updated CHANGELOG.md

/cc @pmalek @czeslavo
Could you PTAL?