Make ordering configurable

[yanniszark]

Resolves #3913

Make kustomize's ordering of resources customizable from the kustomization.yaml file.

My changes in a nutshell:

1. Add the fields to the kustomization struct
2. Extend the LegacyResourceOrderingPlugin to be configurable in its ordering.
3. Wire it up with the new fields.

We may also want to delete the now unused IdSlice code, but I've leaven it up to the reviewer to guide me on this.

cc @KnVerey @monopole

ALLOW_MODULE_SPAN

[k8s-ci-robot]

@yanniszark: 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.

[k8s-ci-robot]

Hi @yanniszark. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[natasha41575]

/ok-to-test

[natasha41575]

As a note, you can run the command make prow-presubmit-check to run all the tests here locally.

[yanniszark]

/retest

[natasha41575]

/retest

[natasha41575]

I might have to unpin some modules for the presubmit to pass. I'll try to make some time to do that tomorrow!

[yanniszark]

@natasha41575 I'm a bit perplexed by what happened.
The presubmit in the PR fails with:

go: sigs.k8s.io/kustomize/plugin/builtin/legacyordertransformer: package sigs.k8s.io/kustomize/kyaml/resid imported from implicitly required module; to add missing requirements, run:
	go get sigs.k8s.io/kustomize/kyaml/resid@v0.11.0
make: *** [Makefile:183: api/builtins/LegacyOrderTransformer.go] Error 1

I pinned it down to the unit tests of the legacy order transformer.
However, this made little sense to me, because that particular module (kyaml) is replaced with the local version.
Perhaps you have more insight on this?

[yanniszark]

BTW, I added a test and fixed the behavior of the plugin to follow these rules:

• If there is no legacySortOptions field set, use the existing legacy sort options.
• If there is a legacySortOptions field present, use its options. orderFirst and orderLast are empty by default.

Test failed because I needed to split changes spanning different modules into separate commits.
Hopefully tests will pass now.

[natasha41575]

I pinned it down to the unit tests of the legacy order transformer.
However, this made little sense to me, because that particular module (kyaml) is replaced with the local version.
Perhaps you have more insight on this?

We ran into this problem too but didn't know why :) but as long as it works now that's fine. I'm running the workflows now, I'll take a closer look at this PR this week.

[natasha41575]

Could you add an e2e test in api/krusty (feel free to create a new file for it, and use the other files in that folder as examples) to demonstrate how one would use this in a kustomization file?

[natasha41575]

EDIT: I need to read through the issue more carefully actually, you can disregard this comment.

I don't know if having two separate top-level fields is necessary, so I commented on the issue.

[yanniszark]

@KnVerey done! Let's see if it passes now :)

[KnVerey]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: KnVerey, yanniszark

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 [KnVerey]

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

[yanniszark]

@natasha41575 @KnVerey many thanks for all the help in getting this change in :)

[KnVerey]

Thank YOU for all your hard work on this feature!

[natasha41575]

I am so happy to see this in! Amazing work and thank you, both of you for seeing this through!

[natasha41575]

@yanniszark Do you have time to write up some docs before this feature is released next Friday 12/16? The docs live here: https://github.com/kubernetes-sigs/cli-experimental/tree/master/site/content/en/references/kustomize/kustomization

If you are unable to, just let us know and we can try to write something up ourselves.

[yanniszark]

@natasha41575 yes I will make sure to do so! I will try to parse the current structure and infer the necessary docs to write, but please feel free to tell me what you would expect as a good set of documentation for this feature.

[dstraub]

In generell, we like this approach.
But how could we use 'fifo' with resources and generators ?
The resources are orderd, but configmaps/secrets created by generators appears at the end.

We have only one exception the the 'legacy' order: Node isn't in the legacy list and comes at least.
But we have to adjust node labels before everything else.

[lindhe]

It says in the documentation:

This field is the endorsed way to sort resources. It should be used instead of the --reorder CLI flag, which is deprecated.

It is a bit unclear to me if that just means that it is the endorsed way to set the ordering options (instead of the --reorder flag), or if it is the endorsed way of sorting (i.e. one should set sortOptions.order=fifo to use the new recommended way of working).

Can someone please clarify this to me?