components are applied after generators, before transformers #5170

koba1t · 2023-05-10T19:58:32Z

natasha41575 · 2023-05-26T17:33:48Z

/approve

@annasong20, when you give this PR a review could you also build off the branch to be a second verifier that it fixes #5141?

k8s-ci-robot · 2023-05-26T17:33:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: koba1t, natasha41575

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [natasha41575]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

annasong20

Only readability comments. Great spacing and go.work.sum fixes.

annasong20 · 2023-05-30T04:10:26Z

api/internal/target/kusttarget.go

@@ -238,6 +234,11 @@ func (kt *KustTarget) accumulateTarget(ra *accumulator.ResAccumulator) (
 		return nil, errors.WrapPrefixf(
 			err, "merging vars %v", kt.kustomization.Vars)
 	}
+	// components are accumulated last.


In this comment, can we explain why "components are accumulated last"?

I add a comment at
983165a.
Do you think this comment is enough to explain it?

annasong20 · 2023-05-30T04:23:44Z

api/krusty/component_test.go

+		input          []FileGen
+		expectedOutput string
+	}{
+		"a": {


Can we rename the test to something more descriptive? For example, "after generators".

Oh!
Sorry, I forgot to fix the test name.

I fixed it at 0c38baa.

annasong20 · 2023-05-30T04:37:06Z

@annasong20, when you give this PR a review could you also build off the branch to be a second verifier that it fixes #5141?

Fixes the issue!

k8s-ci-robot · 2023-05-31T16:15:03Z

This PR has multiple commits, and the default merge method is: merge.
You can request commits to be squashed using the label: tide/merge-method-squash

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.

koba1t · 2023-05-31T16:19:29Z

Hi @annasong20

I have done a fix.
Could you recheck it?

natasha41575 · 2023-05-31T17:25:52Z

/hold

For discussion

natasha41575 · 2023-05-31T18:01:45Z

api/internal/target/kusttarget.go

@@ -238,6 +234,12 @@ func (kt *KustTarget) accumulateTarget(ra *accumulator.ResAccumulator) (
 		return nil, errors.WrapPrefixf(
 			err, "merging vars %v", kt.kustomization.Vars)
 	}
+	// components are accumulated last.
+	// components are expected to execute after reading resources, adding generators, and applying transformers.
+	ra, err = kt.accumulateComponents(ra, kt.kustomization.Components)


As discussed in meeting, we decided to move this to after generators, but before transformers

@koba1t I'll review the PR again after this change.

@annasong20
I moved the code and added a test case. 1d0a14d
Could you recheck it?

annasong20 · 2023-06-02T17:14:12Z

/tide merge-method-squash
/lgtm

annasong20 · 2023-06-02T17:19:02Z

/label tide/merge-method-squash
/unhold

ephesused · 2023-06-05T14:37:54Z

FYI, we are seeing some kustomize build failures associated with this commit. I'm working to isolate the source and create a simple test case.

ephesused · 2023-06-05T18:45:40Z

I believe the failures we're seeing are expected given this PR's description, but if someone is willing to confirm, I'd appreciate it. My understanding is that our current pattern (generate a configmap in the component and merge it in the top-level kustomization.yaml) will need to be inverted (generate a configmap in the top-level kustomization.yaml and merge it in the component). Is that accurate?

kustomization.yaml:

components:
- component-example
configMapGenerator:
- files:
  - datafile.txt
  name: cm-for-datafile
  behavior: merge

component-example/kustomization.yaml:

kind: Component

generators:
- generators.yaml

component-example/generators.yaml:

apiVersion: builtin
kind: ConfigMapGenerator
metadata:
  name: cm-for-datafile
labels:
  app.kubernetes.io/name: cm-for-datafile

datafile.txt is empty.

Results from kustomize builds before and after this commit:

[root@e7265ac0feda pr5170]# ../../kustomize-execs/branch-6ce0bf39/kustomize build .
apiVersion: v1
data:
  datafile.txt: ""
kind: ConfigMap
metadata:
  name: cm-for-datafile-hbfc9tkcd5
[root@e7265ac0feda pr5170]# ../../kustomize-execs/branch-0add0f95/kustomize build .
Error: merging from generator &{0xc0004381a0 <nil>}: id resid.ResId{Gvk:resid.Gvk{Group:"", Version:"v1", Kind:"ConfigMap", isClusterScoped:false}, Name:"cm-for-datafile", Namespace:""} does not exist; cannot merge or replace
[root@e7265ac0feda pr5170]#

annasong20 · 2023-06-06T04:02:49Z

My understanding is that our current pattern (generate a configmap in the component and merge it in the top-level kustomization.yaml) will need to be inverted (generate a configmap in the top-level kustomization.yaml and merge it in the component). Is that accurate?

@ephesused Yes, I believe that should solve the problem. Thank you for the detailing your setup and feel free to leave any feedback for this change at #5141.

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [kubernetes-sigs/kustomize](https://togithub.com/kubernetes-sigs/kustomize) | minor | `v5.0.3` -> `v5.1.0` | | [zellij-org/zellij](https://togithub.com/zellij-org/zellij) | patch | `v0.37.0` -> `v0.37.1` | --- ### Release Notes <details> <summary>kubernetes-sigs/kustomize</summary> ### [`v5.1.0`](https://togithub.com/kubernetes-sigs/kustomize/releases/tag/kustomize/v5.1.0) [Compare Source](https://togithub.com/kubernetes-sigs/kustomize/compare/kustomize/v5.0.3...kustomize/v5.1.0) ##### Highlights Components are now applied after generators: [kubernetes-sigs/kustomize#5170 We made this change cautiously as we want to avoid user friction when updating kustomize; however we felt that it is a bug that resources from `resources` and resources from generators are treated differently. Given that components is currently an alpha feature, we felt that now was the time to adjust if ever. We are currently working on on [kubernetes-sigs/kustomize#4402, which will make the ordering of `resources`, `components`, and `generators` customizable in the kustomization file. Please use that issue for tracking if you are blocked from upgrading kustomize for this reason, and we hope to have it available soon. See [kubernetes-sigs/kustomize#5141 for more detailed discussion. ##### Bug fixes: [#5030](https://togithub.com/kubernetes-sigs/kustomize/issues/5030): `kustomize edit` will return an error if there is no matching path [#5170](https://togithub.com/kubernetes-sigs/kustomize/issues/5170): components are now applied after generators ##### Dependency updates [#5186](https://togithub.com/kubernetes-sigs/kustomize/issues/5186): Update gnostic dependency with gnostic-models [#5187](https://togithub.com/kubernetes-sigs/kustomize/issues/5187): Bump to go 1.20 [#5212](https://togithub.com/kubernetes-sigs/kustomize/issues/5212): pin to kyaml v0.14.3 [#5213](https://togithub.com/kubernetes-sigs/kustomize/issues/5213): pin to cmd/config v0.11.3 [#5214](https://togithub.com/kubernetes-sigs/kustomize/issues/5214): pin to api 0.14.0 </details> <details> <summary>zellij-org/zellij</summary> ### [`v0.37.1`](https://togithub.com/zellij-org/zellij/releases/tag/v0.37.1) [Compare Source](https://togithub.com/zellij-org/zellij/compare/v0.37.0...v0.37.1) ##### RELEASE IN PROGRESS Thank you for your patience! </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://togithub.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/scottames/dots).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

seh · 2023-06-20T14:04:13Z

@koba1t, the title of this PR is confusing: It doesn't match the comments in the code, and doesn't match the decisions reached in #5141. As far as I can tell, we're applying components before transformers. Is that correct?

koba1t · 2023-06-20T14:32:46Z

/retitle components are applied after generators, before transformers

koba1t · 2023-06-20T14:33:25Z

@seh

That's right. Thanks!

kubernetes-sigs#5170) * change: components apply after all generator and transformer applied * fix name for a test case * add comment about when the components will be executed * components are applied before transformer

…release Kustomize release 5.1.0 (our CI job uses this version) made the changes in the order of processing of resources, components , generators and transformers. That requires that our top level kustomization yamls should create the configmaps, rather than replace it, because with 5.1.0, components will be processed after generators, so there won't be any existing configmap to replace. Following issue has more details about it: kubernetes-sigs/kustomize#5170 Signed-off-by: Anil Kumar Vishnoi <avishnoi@redhat.com>

Kustomize release 5.1.0 (our CI job uses this version) made the changes in the order of processing of resources, components , generators and transformers. That requires that our top level kustomization yamls should create the configmaps, rather than replace it, because with 5.1.0, components will be processed after generators, so there won't be any existing configmap to replace. Following issue has more details about it: kubernetes-sigs/kustomize#5170 Signed-off-by: Anil Kumar Vishnoi <avishnoi@redhat.com>

change: components apply after all generator and transformer applied

8ef3361

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 10, 2023

k8s-ci-robot requested review from annasong20 and yuwenma May 10, 2023 19:58

koba1t mentioned this pull request May 17, 2023

Components are not "evaluated after the resources of the parent" #5172

Open

natasha41575 requested review from natasha41575 and annasong20 and removed request for annasong20 and yuwenma May 26, 2023 17:33

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 26, 2023

annasong20 mentioned this pull request May 30, 2023

Kustomizations introduced by components are not applied to adjacent generated resources #5141

Closed

annasong20 suggested changes May 30, 2023

View reviewed changes

koba1t added 2 commits June 1, 2023 00:17

fix name for a test case

0c38baa

add comment about when the components will be executed

983165a

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 31, 2023

natasha41575 removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 31, 2023

natasha41575 reviewed May 31, 2023

View reviewed changes

components are applied before transformer

1d0a14d

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 31, 2023

k8s-ci-robot assigned annasong20 Jun 2, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 2, 2023

k8s-ci-robot added tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jun 2, 2023

k8s-ci-robot merged commit 0add0f9 into kubernetes-sigs:master Jun 2, 2023
8 checks passed

koba1t deleted the change/components_apply_after_all_generator_and_transformer_applied branch June 2, 2023 21:57

k8s-ci-robot changed the title ~~change: components apply after all generators and transformers applied~~ components are applied after generators, before transformers Jun 20, 2023

annasong20 mentioned this pull request Aug 5, 2023

configMapGenarator merge behaviour only allows for one override, ignores subsequent overrides #5250

Closed

vishnoianil mentioned this pull request Sep 16, 2023

Update playground/qa manifest to adopt changes from latest kustomize release nexodus-io/nexodus#1414

Merged

vishnoianil mentioned this pull request Sep 28, 2023

Update prod manifest to adopt changes from latest kustomize release nexodus-io/nexodus#1476

Merged

renaudguerin mentioned this pull request Nov 13, 2023

Add 'ignoreMissing' Flag to replacement options to allow opting for pre-5.0.0 behavior #5440

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

components are applied after generators, before transformers #5170

components are applied after generators, before transformers #5170

koba1t commented May 10, 2023

natasha41575 commented May 26, 2023

k8s-ci-robot commented May 26, 2023

annasong20 left a comment

annasong20 May 30, 2023

koba1t May 31, 2023

annasong20 May 30, 2023

koba1t May 31, 2023

koba1t May 31, 2023 •

edited

annasong20 commented May 30, 2023

k8s-ci-robot commented May 31, 2023

koba1t commented May 31, 2023

natasha41575 commented May 31, 2023

natasha41575 May 31, 2023

annasong20 May 31, 2023

koba1t May 31, 2023

annasong20 commented Jun 2, 2023

annasong20 commented Jun 2, 2023

ephesused commented Jun 5, 2023

ephesused commented Jun 5, 2023

annasong20 commented Jun 6, 2023

seh commented Jun 20, 2023

koba1t commented Jun 20, 2023

koba1t commented Jun 20, 2023

components are applied after generators, before transformers #5170

components are applied after generators, before transformers #5170

Conversation

koba1t commented May 10, 2023

natasha41575 commented May 26, 2023

k8s-ci-robot commented May 26, 2023

annasong20 left a comment

Choose a reason for hiding this comment

annasong20 May 30, 2023

Choose a reason for hiding this comment

koba1t May 31, 2023

Choose a reason for hiding this comment

annasong20 May 30, 2023

Choose a reason for hiding this comment

koba1t May 31, 2023

Choose a reason for hiding this comment

koba1t May 31, 2023 • edited

Choose a reason for hiding this comment

annasong20 commented May 30, 2023

k8s-ci-robot commented May 31, 2023

koba1t commented May 31, 2023

natasha41575 commented May 31, 2023

natasha41575 May 31, 2023

Choose a reason for hiding this comment

annasong20 May 31, 2023

Choose a reason for hiding this comment

koba1t May 31, 2023

Choose a reason for hiding this comment

annasong20 commented Jun 2, 2023

annasong20 commented Jun 2, 2023

ephesused commented Jun 5, 2023

ephesused commented Jun 5, 2023

annasong20 commented Jun 6, 2023

seh commented Jun 20, 2023

koba1t commented Jun 20, 2023

koba1t commented Jun 20, 2023

koba1t May 31, 2023 •

edited