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

Cache *Unstructured objects #5338

Closed
negz opened this issue Feb 5, 2024 · 9 comments · Fixed by #5651
Closed

Cache *Unstructured objects #5338

negz opened this issue Feb 5, 2024 · 9 comments · Fixed by #5651
Assignees
Labels
Milestone

Comments

@negz
Copy link
Member

negz commented Feb 5, 2024

What problem are you facing?

Unstructured: false, // this is the default to not cache unstructured objects

The Kubernetes API client we use doesn't cache *unstructured.Unstructured objects unless you ask it to. We currently don't.

A significant amount of what core Crossplane does is reconcile claims, XRs, and composed resources. All of these resources are of arbitrary types, so they're all backed by *unstructured.Unstructured.

Right now I don't think any of the composition controllers are benefiting from caching.

How could Crossplane help solve your problem?

We should investigate whether caching *Unstructured resources is useful.

There is some danger in caching *Unstructured resources. I believe it's off by default because it would be easy to accidentally start caching resources you probably don't want to. For example if Crossplane somehow took a watch on all Pods in a large cluster it might run out of memory to cache them.

I imagine the benefit would primarily be reduced load on the API server. Composition controllers might be a little more performance too, at the expense of higher memory consumption.

@negz negz added enhancement New feature or request performance composition labels Feb 5, 2024
@negz
Copy link
Member Author

negz commented Feb 5, 2024

I could imagine putting this behind a flag (a feature flag?) to let folks experiment.

@jsanda
Copy link

jsanda commented Feb 16, 2024

I have a custom controller written in Go that is trying manage an XRD that I have written. I created Go structs for the XRD for use in my controller. I configure things like this:

func (r *MyReconciler) SetupWithManager(mgr ctrl.Manager) error {
	return ctrl.NewControllerManagedBy(mgr).
		For(&v1alpha1.MyCustomResource{}).
		Owns(&v1alpha1.MyXRD{}).
		Complete(r)
}

If I change the XR or claim generate by my controller, it isn't receiving an event to trigger a reconciliation. Could the reason for this be due to the fact that Crossplane doesn't cache unstructured resources?

@negz
Copy link
Member Author

negz commented Feb 16, 2024

@jsanda Is your controller running as a separate process/pod? If so, Crossplane's caching behavior won't have an effect on it.

@jsanda
Copy link

jsanda commented Feb 16, 2024

Yes, it is running its own pod. I guess the behavior I am seeing is due to something else. Thanks!

@negz
Copy link
Member Author

negz commented Apr 18, 2024

An older, related issue: #2645

@negz
Copy link
Member Author

negz commented May 2, 2024

@enesonus I remembered today that the XR controllers aren't using the "main" controller-runtime cache that I configured to cache *Unstructured resources in #5339. Each XR controller creates its own cache:

https://github.com/crossplane/crossplane-runtime/blob/v1.15.1/pkg/controller/engine.go#L211

We do this because when we wrote the code it wasn't possible to stop an informer (i.e. a cache of a specific kind of resource), so instead we just stopped the entire cache when we shut down the controller.

Though given we're not caching *Unstructured in these XR caches, I wonder whether they're doing basically nothing. 🤔

@turkenh
Copy link
Member

turkenh commented May 6, 2024

How does this relate to "real-time compositions" feature?

We already initialize caches per GVK, probably only to get notified when something changes. Could it be possible to leverage those caches while getting/reading composed resources? 🤔

@negz
Copy link
Member Author

negz commented May 7, 2024

@turkenh Yeah, good question. I actually opened #5651 today to try using a single cache when realtime compositions are enabled. It needs more work though. It doesn't pass E2E tests yet.

@negz
Copy link
Member Author

negz commented May 7, 2024

Could it be possible to leverage those caches while getting/reading composed resources? 🤔

I think we do when realtime compositions are enabled. There's code in runtime that wraps a manager's client with one backed by the GVK routed cache.

@jbw976 jbw976 added this to the v1.17 milestone May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment