Support a high-level merge behavior in kustomize

[monopole]

The resources: field in a kustomization file specifies a list of files and / or other kustomizations to be processed in a kustomize build.

These artifacts are read or built, resulting in a list of resources, which are supposed to be added to the set of resources kustomize is processing.

If an incoming resource matches the Id (where Id is a tuple of namespace, group, version, kind, name) of an Id already known to kustomize, the build fails with an error. This invariant maintains the Id is a primary key used for all sorts of purposes.

We could however, allow a merge on Id collision instead of an error (at this stage of the build process). Or we could make merge the default behavior, and have some mode that enables errors on Id collisions.

Some uses cases prefer an error, some prefer a merge.

There are low level cases where this is already allowed, e.g. Secret and ConfigMap generators have an option that allows merging or replacing on Id collision, rather than failing.

[monopole]

take into account

• build wide resolution, (e.g. a flag to build), per kustomization resolution, or even per Kind resolution (as we already do with ConfigMap and Secret being different from other Kinds) - i.e. always merge Deployments, but never merge/replace Namespace.
• leave resources alone, and make a new field that does merging?
• reuse https://github.com/kubernetes-sigs/kustomize/blob/master/pkg/types/generationbehavior.go generally, or deprecate in favor of something else?

[monopole]

generationbehavior defines three behaviors for generators

• create: make a new thing, and throw an error if it already exists.
• replace: make a new thing, expect to replace an existing thing, and error if the existing thing isn't there
• merge: make a new thing, and expect to merge it with an existing thing, and error if the existing thing isn't there.

That logic is in ResMap.AbsorbAll, but is not invoked when reading the resources field.

So how about a new field that offer all flexibility:

sources: []Source
type Source {
  behavior: create, replace or merge or ...
  resources: []string
}

e.g.

sources:
- behavior: merge
  resources:
  - a/path/to/file
  - a/kustomization
- behavior: create
  resources:
  - b/path/to/file
  - b/kustomization
- behavior: replace
  resources:
  - c/path/to/file
  - c/kustomization

The existing resources field, kept for backward compatibility, is thus shorthand for

sources:
- behavior: create
  resources:
  - some/path/to/file
  - some/kustomization

This way future behaviors can be identified, e.g. override which is like createOrReplace (no error)

[apyrgio]

Allowing the user to specify a conflict resolution strategy for resources is a nice addition. To get the discussion started, we have the following comments:

1. Why not keep the resources as is, e.g.:

resources:
- a/path/to/file
- a/kustomization
- b/path/to/file
- b/kustomization
- c/path/to/file
- c/kustomization

and also allow this format:

resources:
- source: a/path/to/file
  behavior: merge
- source: a/kustomization 
  behavior: merge
- source: b/path/to/file
  behavior: create
- b/kustomization  # This format is also allowed,
                   # with "create" as the default behaviour
- source: c/path/to/file
  behavior: replace
- source: c/kustomization
  behavior: replace

The default behavior is currently create, so users will rarely need to write the full format. Also, this format allows us to add more fine-grained control for resources in the future, and the user does not need to group resources per behavior.

2. The merging strategy in generators is simple to reason out, since they deal with created resources. In the case of patching the same resource from different overlays, how do you plan to implement merging? From what I gather, merging the produced YAMLs can overwrite changed fields (see @jbrette's solution), and recreating the patches is very tricky.

[apyrgio]

Minor bump for this issue. Is someone working on @monopole's suggestion? I'm asking because I see that there's a related PR of @jbrette that is currently blocked, citing this issue as an alternative.

In the meantime, there was another request by a different user for roughly the same functionality (#1372). Since this is a recurring request, I think it would help if we initially decide on what approach to follow, now that the discussion regarding this issue is still warm.

[ioandr]

Hi @monopole,

this is a heads-up to keep this issue active.

Is there any progress on the implementation of this feature or any feedback on the format proposed by @apyrgio in #1292 (comment) ?

Plus, a kind reminder for #1297, which describes our case and currently fails. It would be nice to make it work as expected and serve as the baseline example for the higher level merge functionality.

[monopole]

The issue still open. been doing other things - and also hoping for more evidence of interest.

This has lots of overlap with patch behavior, as any resource file is also a valid patch.

We cannot precisely do as #1292 (comment) suggests. The kustomization file is a Go struct for standard marshalling/unmarshalling; the resources field has to be an array of string. If it becomes an array of struct, we're creating a backward incompatible format. The two interpretations cannot co-exist at the same value of the apiVersion field.

We can do a new field though, e.g. sources as mentioned in #1292 (comment).

The other work here, besides the relatively easy work in changing how kustomize treats the files in a build, is providing clear guidance on how to use the patch field (as there would now be two ways to patch), and working out how the editsub- commands would work.

[monopole]

@ioandr with respect to #1297, PTAL at #1615

I think this, is or is close to, your desired result, with a different approach that doesn't require high-level merge behavior.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.