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 #3672

Open
mortent opened this issue Nov 23, 2022 · 2 comments
Open

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

mortent opened this issue Nov 23, 2022 · 2 comments
Assignees
Labels
area/porch bug Something isn't working triaged Issue has been triaged by adding an `area/` label

Comments

@mortent
Copy link
Contributor

mortent commented Nov 23, 2022

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 #3593, this should be pretty straightforward to handle in the client.

@mortent mortent added bug Something isn't working triaged Issue has been triaged by adding an `area/` label area/porch labels Nov 23, 2022
@mortent
Copy link
Contributor Author

mortent commented Nov 23, 2022

@justinsb @ChristopherFry @natasha41575 Any thoughts on this?

@mortent mortent self-assigned this Nov 23, 2022
@natasha41575
Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/porch bug Something isn't working triaged Issue has been triaged by adding an `area/` label
Projects
None yet
Development

No branches or pull requests

2 participants