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

kpt.dev/latest-revision label doesn't work well with controllers #607

Open
liamfallon opened this issue Apr 8, 2024 · 1 comment
Open
Labels
area/porch Porch related issues bug Something isn't working triaged

Comments

@liamfallon
Copy link
Member

Original issue URL: kptdev/kpt#3672
Original issue user: https://github.com/mortent
Original issue created at: 2022-11-23T12:56:53Z
Original issue last updated at: 2023-01-28T01:27:21Z
Original issue body: We use the kpt.dev/latest-revision label to tell clients which PackageRevision is the latest for a package. This works well when users use the LIST command so we can always provide a snapshot of all PackageRevisions from the same point in time. But for controllers that rely on watches to build a local cache of PackageRevisions, this doesn't work, since publishing a new PackageRevision that becomes the latest one doesn't notify the clients that the kpt.dev/latest-revision label on other revisions are no longer correct.

One possible solution would be that we send watch updates for any PackageRevisions that are no longer the latest and thereby make sure that controller caches get updated correctly. However, this will lead to a race that can lead to controllers for short period of times can either have two PackageRevisions marked as latest, or have no PackageRevision marked as latest. This depends on whether we chose to send the watch events for the new or previous latest PackageRevision first. Also, this approach is difficult to manage with CRDs, since it doesn't give us a good way to do cross-PackageRevision logic.

So I think our best alternative is to drop the kpt.dev/latest-revision label and rely on clients to use the revision field to determine which PackageRevision is the latest. After kptdev/kpt#3593, this should be pretty straightforward to handle in the client.

Original issue comments:
Comment user: https://github.com/mortent
Comment created at: 2022-11-23T12:57:26Z
Comment last updated at: 2022-11-23T12:57:26Z
Comment body: @justinsb @ChristopherFry @natasha41575 Any thoughts on this?

Comment user: https://github.com/natasha41575
Comment created at: 2022-11-28T18:36:18Z
Comment last updated at: 2022-11-28T18:36:18Z
Comment body: I think asking the client to use the revision field to determine the latest revision is reasonable. Perhaps we could provide a library function in the kpt repo that does the bulk of the logic, for clients to call?

@tliron tliron transferred this issue from nephio-project/porch-issue-transfer Apr 23, 2024
@liamfallon
Copy link
Member Author

liamfallon commented May 21, 2024

Triage comment:
Needs more investigation on how to replicate this. Consider in rearchitecture work. This makes it harder to implement controllers that are watching Porch PackageRevision CRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/porch Porch related issues bug Something isn't working triaged
Projects
None yet
Development

No branches or pull requests

1 participant