teach fn render to prune resources #1589
Conversation
|
|
||
| // pruneResources compares the input and output of the hydration and prunes | ||
| // resources that are no longer present in the output of the hydration. | ||
| func pruneResources(hctx *hydrationContext) error { |
There was a problem hiding this comment.
presumably, this functionality exists in current kpt fn run. What's the plan for converging this? We need pruning for imperative functions as well.
There was a problem hiding this comment.
One thing to note is that during hydration, we read resources as we traverse the pkg hierarchy (and also gather resources by applying pipeline) and write it finally in the end. The read/write flow is very different from imperative run or just pkg level operation. For pruning, you want to track resources at reading time and writing time, so the flow will be different for hydration vs imperative workflow.
For ex, imperative runs etc. uses kio.LocalPkgReaderWriter https://github.com/kubernetes-sigs/kustomize/blob/kyaml/v0.10.15/kyaml/kio/pkgio_reader.go#L116 where it tracks the file while .Read() and prunes files while Write(resources) call. So that is useful at the pkg level, but isn't suited to be used for the hydration workflow.
What do you have in mind when you think of converging ?
There was a problem hiding this comment.
I'm not following why declarative and imperative are different when it comes to package io and prunning. There are 3 high level stages:
- Get the current set of input resources across all subpackages. This can be done in different ways. a) Aggregate
inputResourcesas part of recursive algorithm b)Read()on top-level package withincludeSubpackages - Runs functions as part recursive hydration to calculate the resultant resources across all packages. This is the
outputResources. - Write the
outputResourcesto disk across all packages with pruning (diff step 1 and 3).
The only difference between imperative and declarative is step 2, no? Pruning is a concern for package abstraction (may use LocalPkgReaderWriter as impl detail) , and not the hydration process. It would be weird to duplicate the logic in step 3.
There was a problem hiding this comment.
For pruning to work, we need to track the input resources (A) and output resources (B) so that we can compute the difference A-B at the end and perform the deletion from FS. Keeping track of input as we read resources during imperative or declarative flow is where we need the right abstraction so that we can de-dupe the logic for pruning. One way to do it will be have some sort of resourceTracker that you can provide to the abstractions pkg.LocalReaderWriter() or hydrationContext so that these methods could call resourceTracker.Track(record) as they read resource or feed to the function chains and at the end have some sort of resourceTracker.Reconcile(output) to get the difference between A-B and do the pruning.
Would like to have a chat about this. Scheduling 30 mins tomorrow to go over this.
There was a problem hiding this comment.
Can you file an issue to follow up on this. I'm still missing something given current implementation of declarative flow dooesn't reimplement prunning.
There was a problem hiding this comment.
Filed an issue here #1609 to track this conversation.
|
|
||
| // pruneResources compares the input and output of the hydration and prunes | ||
| // resources that are no longer present in the output of the hydration. | ||
| func pruneResources(hctx *hydrationContext) error { |
There was a problem hiding this comment.
I'm not following why declarative and imperative are different when it comes to package io and prunning. There are 3 high level stages:
- Get the current set of input resources across all subpackages. This can be done in different ways. a) Aggregate
inputResourcesas part of recursive algorithm b)Read()on top-level package withincludeSubpackages - Runs functions as part recursive hydration to calculate the resultant resources across all packages. This is the
outputResources. - Write the
outputResourcesto disk across all packages with pruning (diff step 1 and 3).
The only difference between imperative and declarative is step 2, no? Pruning is a concern for package abstraction (may use LocalPkgReaderWriter as impl detail) , and not the hydration process. It would be weird to duplicate the logic in step 3.
droot
force-pushed
the
fn-render-resource-pruning
branch
from
948d709
to
04ea2d4
Compare
| func pruneResources(hctx *hydrationContext) error { | ||
| filesToBeDeleted := hctx.inputFiles.Difference(hctx.outputFiles) | ||
| for f := range filesToBeDeleted { | ||
| if err := os.Remove(filepath.Join(string(hctx.root.pkg.UniquePath), f)); err != nil { |
There was a problem hiding this comment.
Same discussion as Donny's PR about semantics of path annotation.
There was a problem hiding this comment.
We realized we need to address this problem across the codebase (especially hydration that deals with path annotation much more). This code uses the kioutl.GetPathAnnotation abstraction to get the path annotation. If this abstraction returns OS-specific path eventually (by doing the conversion from OS-agnostic to OS-specific...thinking of this abstraction having os-specific path_{linux, windows}.go), rest of the code will be using OS-specific code as today ('filepath.Join(..)`).
@Shell32-Natsu filed an issue kubernetes-sigs/kustomize#3749 to track this in kyaml library.
|
|
||
| // pruneResources compares the input and output of the hydration and prunes | ||
| // resources that are no longer present in the output of the hydration. | ||
| func pruneResources(hctx *hydrationContext) error { |
There was a problem hiding this comment.
Can you file an issue to follow up on this. I'm still missing something given current implementation of declarative flow dooesn't reimplement prunning.
This PR adds support for pruning of resources in hydration.
fixes #1524
Note: Error reporting is still not in-place. I will address all error reporting issues related to hydration in one PR after this PR.