Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

master branch orders some resource names differently than v4.4.1 #4388

Closed
ephesused opened this issue Jan 14, 2022 · 11 comments · Fixed by #4445
Closed

master branch orders some resource names differently than v4.4.1 #4388

ephesused opened this issue Jan 14, 2022 · 11 comments · Fixed by #4445
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@ephesused
Copy link
Contributor

ephesused commented Jan 14, 2022

Describe the bug

At v4.4.1 and before, kustomize build produces output with resource names sorted like so:

  1. testing
  2. testing-one
  3. testing-two

In the master branch (I believe first appearing at 92197fd), kustomize build produces output with resource names sorted like so:

  1. testing-one
  2. testing-two
  3. testing

Cross reference #4361 (comment)

Files that can reproduce the issue

kustomization.yaml

resources:
  - resources.yaml

resources.yaml

apiVersion: v1
kind: ConfigMap
metadata:
  name: testing
data:
  key: value
---
apiVersion: v1
kind: ConfigMap
metadata:
  name: testing-one
data:
  key: value
---
apiVersion: v1
kind: ConfigMap
metadata:
  name: testing-two
data:
  key: value

Expected output, using v4.4.1 as an example

$ build/kustomize-execs/release-v4.4.1/kustomize version
{Version:kustomize/v4.4.1 GitCommit:b2d65ddc98e09187a8e38adc27c30bab078c1dbf BuildDate:2021-11-11T23:36:27Z GoOs:linux GoArch:amd64}
$ build/kustomize-execs/release-v4.4.1/kustomize build testdata/kust-resource-sort
apiVersion: v1
data:
  key: value
kind: ConfigMap
metadata:
  name: testing
---
apiVersion: v1
data:
  key: value
kind: ConfigMap
metadata:
  name: testing-one
---
apiVersion: v1
data:
  key: value
kind: ConfigMap
metadata:
  name: testing-two
$

Actual output

$ cat build/kustomize-execs/branch-master/info.yaml
branch: master
sha1: 6e82b210a9e0651cc8392d2931c64348137b82ff
$ build/kustomize-execs/branch-master/kustomize build testdata/kust-resource-sort
apiVersion: v1
data:
  key: value
kind: ConfigMap
metadata:
  name: testing-one
---
apiVersion: v1
data:
  key: value
kind: ConfigMap
metadata:
  name: testing-two
---
apiVersion: v1
data:
  key: value
kind: ConfigMap
metadata:
  name: testing
$

Kustomize version

This change is not (yet) in a released version. I see the change in the master branch, and I believe it first appears at 92197fd.

Platform

Linux

@ephesused ephesused added the kind/bug Categorizes issue or PR as related to a bug. label Jan 14, 2022
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jan 14, 2022
@ephesused ephesused changed the title master branch orders resource names differently than v4.4.1 master branch orders some resource names differently than v4.4.1 Jan 14, 2022
@natasha41575
Copy link
Contributor

Are you absolutely certain this behavior started showing up in 92197fd? I’m not sure how that could be the case because that PR doesn’t touch any of the code for kustomize build, it only adds a new sub command to kustomize edit.

Unfortunately I don’t have a laptop with me and won’t be able to verify myself for a few more days.

@ephesused
Copy link
Contributor Author

ephesused commented Jan 14, 2022

When I build off ff7b2f2 (which I believe is the commit immediately prior to 92197fd), I see the v4.4.1 behavior. When I build off 92197fd, I see the changed behavior.

I'd appreciate someone double checking me here - it may be that I'm making a bad assumption or my builds are somehow flawed.

Edited to add: This analysis is incorrect. My mistake was that ff7b2f2's parent is b28f1e5 (not 92197fd).

@ephesused
Copy link
Contributor Author

Poking around some more, I now believe it's 233f1a3. Looking at only the merge pull request commits marked as verified, I see that 75de98e matches v4.4.1, while 233f1a3 has the new sorting behavior.

My mistake was using ff7b2f2 as a comparison point. It's not a merge.

I've been out for a while, so everything is fuzzy. Apologies for my confusion.

@natasha41575
Copy link
Contributor

Thank you, I have been able to reproduce this and can confirm that there is a different order from 4.4.1 and master. We should look into this before the next release.

/triage accepted
/kind bug

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 21, 2022
@natasha41575
Copy link
Contributor

A possible reason is that perhaps in changing Gvk.String, we changed the output of Gvk.IsLessThan.

@ephesused
Copy link
Contributor Author

Just a quick update to note that v4.5.0 includes this sequence change.

@seh
Copy link
Contributor

seh commented Feb 2, 2022

As noted in the "kustomize" Slack channel, in my examples, it appears that kustomize is now sorting by "metadata.labels" before "metadata.name."

Reading further, that's not quite right either. I can't figure out the ordering within a given GVK ("apiVersion" and "kind") within a given Kubernetes namespace ("metadata.namespace").

@ephesused
Copy link
Contributor Author

@seh, I haven't confirmed, but my uneducated guess is that the sorting is happening by way of ResId's String(), which means the sorting change would come from this change.

Assuming I'm reading everything correctly, I believe that would explain the test case in this ticket. The previous behavior was GVK, pipe, namespace, pipe, name. The new behavior is GVK, slash, name, period, namespace. So the previous would been sorting:

  • GVK|namespace|testing
  • GVK|namespace|testing-one
  • GVK|namespace|testing-two

The new would be sorting:

  • GVK/testing.namespace
  • GVK/testing-one.namespace
  • GVK/testing-two.namespace

The previous behavior was dealing with the hyphen (in "testing-one" and "testing-two") as a comparison based on a longer string. So "testing" comes before "testing-one" and "testing-two". The new behavior deals with the hyphen by determining it sorts before a period. So "testing-one" and "testing-two" come before "testing".

Again, the above analysis is based on a cursory review - I'm not certain that this is the cause.

@seh
Copy link
Contributor

seh commented Feb 2, 2022

Ed, that's an excellent explanation. I stared at my diff for about five minutes, trying various explanations in my head, and none came any closer than "2-3-1," meaning that it was almost reversed, but the first and second items were swapped. Your explanation addresses the role of the hyphens in the names.

My examples are triples like "cert-manager," "cert-manager-cainjector," and "cert-manager-webhook." They now sort as follows:

  • cert-manager-cainjector
  • cert-manager-webhook
  • cert-manager

You have now explained why that's so.

@KnVerey KnVerey self-assigned this Feb 2, 2022
@KnVerey
Copy link
Contributor

KnVerey commented Feb 2, 2022

Thanks for the report and detailed reproduction steps. I'm really sorry that we released this regression. I have a PR up to fix it, added a note about it to the release 4.5 notes, and we will release 4.5.1 ASAP today.

@ephesused
Copy link
Contributor Author

Thanks, @KnVerey. I see the fix in v4.5.1. Everything I run comes back exactly the same between v4.4.1 and v4.5.1.

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this issue Dec 6, 2022
Changes:
4.5.7
-----
Due to an oversight, kustomize v4.5.6 has the golang testing library
compiled in unnecessarily. This is a rerelease with the same
functionality, but without the unnecessary additional library compiled
in.

4.5.6
-----
Due to an oversight, kustomize v4.5.6 has the golang testing library
compiled in unnecessarily. It is advised that you upgrade to v4.5.7,
which doesn't have the testing library compiled in.

4.5.5
-----
This release is expected to have significant performance improvements
for a good portion of inputs, due to #4568.

4.5.4
-----
Dependencies updates.

4.5.3
-----
Misc enhancements and bug fixes.

4.5.2
-----
Bug fix release.

4.5.1
-----
Known issues:
 - kubernetes-sigs/kustomize#4455
   A regression, some HTTP urls are not working properly.

4.5.0
-----
 - This release contains a regression in the legacy sort order. Those
   using the legacy sort, i.e. `kustomize build` with `--reorder` unset
   or explicitly set to `legacy`, are advised to skip this release.
   kubernetes-sigs/kustomize#4388
 - kubernetes-sigs/kustomize#4455
   Another regression, some HTTP urls are not working properly.

 - New field in kustomization, `buildMetadata`.
 - New command `kustomize edit add buildmetadata`
 - Refactor the PrefixSuffixTransformer into separate prefix- and
   suffix transformers, enabling the user to use the PrefixTransformer or
   SuffixTransformer individually in the transformers field.
 - `kustomize build ...` now completes file paths on ZSH.
 - New command `kustomize edit add generator` (kubernetes-sigs/kustomize#4361)

 - Deprecate enable-managedby-label flag in favor of a field

4.4.1
------
Bug fix release.

4.4.0
-----
The headline feature of this release is improved support for YAML
anchors and aliases, which will be expanded by default as of this
version.

Additional features and fixes include:
- Added length check on originalFields of kustomizationFile to prevent
  panic when kustomization file began with a comment(or a blank line)
  followed by a document separator

4.3.0
-----
Misc bug fixes and enhancements.

4.2.0
-----
New experimental command to automatically migrate `vars` to
`replacements`: `kustomize edit fix —vars`. For details, run `kustomize
edit fix -h`. Warning: converting `vars` to `replacements` will
potentially overwrite many resource files and in rare scenarios may not
produce the same output when `kustomize build` is run. We recommend
doing this in a clean git repository where the change is easy to undo.

4.1.3
-----
* New experimental ReplacementTransformer, docs on the way:
  kubernetes-sigs/cli-experimental#158
  This will replace the `vars` feature.
* Allow pulls of openapi data from live API servers (openapi fetch command).
* Remote git urls can specify a timeout parameter.
* More examples of helm usage.
* Speed up cluster-scoped type checks.
* API changes towards 1.0
  - `Gvk` and `Resid` types moved to kyaml
  - `Resource` now inlines `RNode` rather than delegating to it
  - `Resmap` now accepts an `kio.Filter` visitor (that can change the ResMap size).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants