Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add KEP for Kustomize components #1

Merged
merged 1 commit into from
May 20, 2020
Merged

Conversation

apyrgio
Copy link

@apyrgio apyrgio commented May 18, 2020

This is the draft area where we will shape up the KEP for Kustomize components. Once done, we can open a PR to kubernetes/enhancements. Rendered KEP here.

Note: I've added FIXMEs where we need to change something, and the guidelines for the KEP in block quotes. We can remove these as we finalize each section.

@apyrgio apyrgio force-pushed the feature-kustomize-components branch from a681aae to fadd817 Compare May 18, 2020 15:06
@apyrgio
Copy link
Author

apyrgio commented May 18, 2020

@ioandr / @pgpx: 👋. This is the PR I was talking about. Let's add suggestions on each section as comments, and I'll periodically commit and push them in the PR.

EDIT: I suggest we start with:

@apyrgio: Proposal, Drawbacks, Alternatives
@ioannis: Summary, Motivation
@pgpx: Design details

and see how it goes. @pgpx, if you have written something that does not follow the KEP's format, share it anyways, and I'll convert it accordingly.

@pgpx
Copy link

pgpx commented May 18, 2020

Design details:

When Kustomize process a Kustomization file it uses a ResourceAccumulator (RA) object, which represents the accumulated state of the processing up to that point (including files, patches, transformers, variables, etc). The RA is initially empty, and then:

  1. Adds each of its resources (in left-to-right order) to the RA (kusttarget.accumulateResources):
    1.1 Adds files to the RA directly (accumulateFile).
    1.2 Recursively processes Kustomizations, each starting with a new empty RA, and with the result merged in to the RA of the parent (accumulateDirectory and resaccumulator.MergeAccumulator).
  2. Adds/merges its CRDs, generators, transformers, and variables to the RA.
    2.1 These modifications can only be applied to entities within the RA - it is an error otherwise. This means that sibling Kustomizations are entirely independent of each other.

A Component is implemented similarly, but instead of starting with an empty RA, it instead takes the RA from its parent at the point at which the Component is processed. This means that some/all of the parent's resources will be in the RA, and so will be modifiable by the component (other elements of the parent will not be present (CRDs, generators, etc.) - they will only be applied when the parent's processing continues). Nested components will work in the same way - ownership of the parent's RA will simply be passed down.

This was implemented by changing the signature of kusttarget.accumulateResources to return a pointer to the ResourceAccumulator that the parent should use, and giving child components their parent's RA instead of an empty RA. This minimised the amount of code changed to simplify review, but a slightly more extensive refactoring would be recommended.

@apyrgio
Copy link
Author

apyrgio commented May 19, 2020

(I suggest we move this to a review thread, so that comments for different sections don't interleave)

Here are my comments on the Design details section:

  1. This means that sibling Kustomizations are entirely independent of each other.

    Maybe also add here that Kustomizations with the same base will cause an error when the resources are accumulated, unless their GVKN is altered? This is basically the main limitation we want to workaround.

  2. A Component is implemented similarly, but instead of starting with an empty RA, it instead takes the RA from its parent at the point at which the Component is processed.

    Can we be more specific about what's exactly the point when the component is evaluated? If I understand correctly, first comes the evaluation of the resources field, then comes the evaluation of the components field, and the RA is passed from component to component, in left-to-right order, correct?

  3. I see in your PR that you have some logic regarding missing apiVersion/kind fields. Maybe add it here as well?

  4. Not sure from the PR, but components can be loaded from any place where one expects to load a kustomization, correct? I mean tarballs, URLs, etc.

Finally, some minor rewordings:

  • When Kustomize process -> When Kustomize processes
  • is initially empty, and then: -> is initially empty, and then Kustomize:
  • it instead takes the RA from its parent -> it takes the RA from its parent

@apyrgio
Copy link
Author

apyrgio commented May 19, 2020

So, I guess we're done with the first round of comments. I just merged everything into the Markdown document. For the second round of comments (if necessary), you can comment on a paragraph directly.

@apyrgio apyrgio force-pushed the feature-kustomize-components branch 4 times, most recently from 457e29b to d39e640 Compare May 20, 2020 08:59
@apyrgio
Copy link
Author

apyrgio commented May 20, 2020

The second round of comments is complete, and all sections are now filled. This is close to the final KEP so please, give it one more full read and comment where necessary.

@apyrgio
Copy link
Author

apyrgio commented May 20, 2020

The third round of comments is complete as well. I'll send the PR in about an hour, so last call for comments is now.

@apyrgio apyrgio force-pushed the feature-kustomize-components branch 2 times, most recently from a6707c0 to 1e5eabc Compare May 20, 2020 11:01
@apyrgio apyrgio force-pushed the feature-kustomize-components branch 3 times, most recently from fbdc466 to 25b0766 Compare May 20, 2020 11:15
This enhancement proposal introduces components, a new kind of
Kustomization that allows users to define reusable kustomizations.

Main discussion: kubernetes-sigs/kustomize#1251
User story that spawned this KEP: kubernetes-sigs/kustomize#2438
@apyrgio apyrgio force-pushed the feature-kustomize-components branch from 25b0766 to 446b675 Compare May 20, 2020 12:16
@apyrgio apyrgio merged commit 35e9662 into master May 20, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants