search

nephio-project / nephio

Nephio is a Kubernetes-based automation platform for deploying and managing highly distributed, interconnected workloads such as 5G Network Functions, and the underlying infrastructure on which those workloads depend.
Apache License 2.0
104 stars 53 forks source link

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

Open liamfallon opened 5 months ago

liamfallon commented 5 months ago

Original issue URL: https://github.com/kptdev/kpt/issues/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 https://github.com/GoogleContainerTools/kpt/pull/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?

liamfallon commented 4 months ago

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.