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

Only watch, don't poll, composite resources #4316

Closed
negz opened this issue Jul 12, 2023 · 9 comments
Closed

Only watch, don't poll, composite resources #4316

negz opened this issue Jul 12, 2023 · 9 comments
Assignees
Milestone

Comments

@negz
Copy link
Member

negz commented Jul 12, 2023

What problem are you facing?

The XR controller currently watches the XR type, but is also poll-triggered, by default polling desired state every 60 seconds. This interval can be changed by the --poll-interval flag. The XR reconciler is poll-triggered because it wants to know when composed resources change, in order to correct drift.

An XR controller doesn't know what kinds of resources it will compose at start time, which is typically when a controller's watches are configured. Furthermore, two different XRs of the same kind might compose completely different types of resources due to using different Compositions.

How could Crossplane help solve your problem?

Ideally XR reconciliation would be purely watch-triggered. This would result in fewer invocations of the XR reconciler and thus presumably the ability for a particular Crossplane deployment to handle more XRs concurrently. This is also particularly important for Composition Functions, where every XR reconcile may call one or more Functions.

@negz
Copy link
Member Author

negz commented Jul 12, 2023

Thinking out loud, it may be possible to:

  1. Make one controller responsible (only) for selecting a Composition for XRs that don't have one.
  2. Start another controller for every unique (XR, Composition) tuple.
  3. Restart the (XR, Composition) controller whenever the XR's spec.resourceRefs changes

The idea being that each time the (XR, Composition) controller was restarted it would take a watch on all of the (types of?) composed resources that appeared in its spec.resourceRefs array.

@negz
Copy link
Member Author

negz commented Jul 12, 2023

FWIW while this relates to Functions I'm not ready to say this is a blocker for Functions going to beta or even GA at this point. I'd like to see some indication that it's a real constraint before we spend time optimizing.

@bobh66
Copy link
Contributor

bobh66 commented Jul 12, 2023

@negz How would this account for the deletion of a composed resource? I don't think that deletion of a composed resource would trigger an event on the owning composite? The poll interval will catch this scenario and recreate the composed resource.

@negz
Copy link
Member Author

negz commented Jul 12, 2023

I don't think that deletion of a composed resource would trigger an event on the owning composite?

I believe it would as long as the controller of the owning composite was watching whatever type of resource was deleted. e.g. If we can notice that the XR has composed a Subnet and restart its controller to start watching for Subnet composed resources, the XR would have a reconcile queued any time any Subnet was deleted.

@github-actions
Copy link

Crossplane does not currently have enough maintainers to address every issue and pull request. This issue has been automatically marked as stale because it has had no activity in the last 90 days. It will be closed in 14 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.

@github-actions github-actions bot added the stale label Oct 11, 2023
@negz
Copy link
Member Author

negz commented Oct 12, 2023

/fresh

@sttts is actively working on this in #4637

@github-actions github-actions bot removed the stale label Oct 12, 2023
@negz
Copy link
Member Author

negz commented Oct 12, 2023

@sttts PS do you want to add this to the v1.14 milestone? I know its behind a feature flag so it would land as alpha.

@sttts
Copy link
Contributor

sttts commented Oct 13, 2023

yes, 1.14 is the plan.

@sttts sttts added this to the v1.14 milestone Oct 16, 2023
@negz
Copy link
Member Author

negz commented Oct 17, 2023

I think we can call this fixed by #4637. In practice we still watch and poll, but I could imagine figuring out removing polling being part of the work to get the 'realtime Compositions' feature to beta in #4828.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

3 participants