liamfallon
commented
4 months ago Triage Comments: In the case of a PackageVariant, it will clone a package that has no readiness gate in it and it seems fine. Then as the package moves through the pipeline, there is no way to track whether of how much of the mutation in the pipeline has been executed. The PacakgeRevision creation is a 2 step process:
- Clone the raw Package
- Mutate the package with a pipeline or otherwise
The current implementation sets the package as ready after step 1.
To be considered in rearchitecture.
Original issue URL: https://github.com/kptdev/kpt/issues/3979 Original issue user: https://github.com/johnbelamaric Original issue created at: 2023-06-01T01:36:52Z Original issue last updated at: 2023-07-06T21:56:34Z Original issue body: ### Expected behavior If PackageVariant fails to inject or otherwise properly mutate the Draft it creates, we need to prevent the package from being approved. This could be done by adding a False condition and readinessGate immediately after cloning (or better yet, implementing a porch server feature that allows that to be done atomically).
Actual behavior
Because we cannot atomically "clone and apply PV mutations", the actual package revision process happens in two independent stages: clone, then apply mutations. If applying the mutations fails, the newly cloned Draft is still there, and unless you check the PV conditions, it looks fine and can be merged (in fact the auto-approval controller we built for Nephio goes ahead and approves it, since it only inspect the package revision, and doesn't know where it came from).
Original issue comments: Comment user: https://github.com/johnbelamaric Comment created at: 2023-07-06T21:56:34Z Comment last updated at: 2023-07-06T21:56:34Z Comment body: As a workaround for this, the approval controller now checks the PV readiness status, and so do some of our other controllers. But that wouldn't help a human user.