kustomize: kustomize re-orders config causing fail when crd or admission webhook used

If we look @ cert-manager we see that it has in its output:

crd
objects
webhooks
objects using webooks + crd

thus the order must be preserved.

So if I apply kustomize across their input and add

---
apiVersion: certmanager.k8s.io/v1alpha1
kind: ClusterIssuer
metadata:
  namespace: kube-system
  name: selfsigning-issuer
spec:
  selfSigned: {}

to the ‘end’, i get a failure

Error from server (InternalError): error when creating "STDIN": Internal error occurred: failed calling webhook "clusterissuers.admission.certmanager.k8s.io": the server could not find the requested resource

the attached output file shows. The selfsigning-issuer ClusterIssuer is emitted in the middle, before its webhook is defined

cert-manager-to-be-applied.yaml.gz

About this issue

Original URL
State: closed
Created 5 years ago
Reactions: 11
Comments: 34 (20 by maintainers)

Commits related to this issue

Order ValidatingWebhookConfig last. Fixes the cert-manager example of #821. — committed to mgoltzsche/kustomize by mgoltzsche 5 years ago
Order ValidatingWebhookConfig last. Fixes the cert-manager example of #821. — committed to mgoltzsche/kustomize by mgoltzsche 5 years ago
Cover input order == output order with tests. Modify all test coverage to compare actual build output against a non-sorted version of expected output. Expected output is specified in the same order ... — committed to monopole/kustomize by monopole 5 years ago
Cover input order == output order with tests. Modify all test coverage to compare actual build output against a non-sorted version of expected output. Expected output is specified in the same order ... — committed to monopole/kustomize by monopole 5 years ago
Modify tests to present expected data in unsorted order. Modify all `build` tests to use the raw, non-sorted output of build. This makes every test provide coverage for how kustomize re-orders (or d... — committed to monopole/kustomize by monopole 5 years ago
Modify tests to present expected data in unsorted order. Modify all `build` tests to use the raw, non-sorted output of build. This makes every test provide coverage for how kustomize re-orders (or d... — committed to monopole/kustomize by monopole 5 years ago
Modify tests to present expected data in unsorted order. Modify all `build` tests to use the raw, non-sorted output of build. This makes every test provide coverage for how kustomize re-orders (or d... — committed to monopole/kustomize by monopole 5 years ago
Modify tests to present expected data in unsorted order. Modify all `build` tests to use the raw, non-sorted output of build. This makes every test provide coverage for how kustomize re-orders (or d... — committed to monopole/kustomize by monopole 5 years ago
Modify tests to present expected data in unsorted order. Modify all `build` tests to use the raw, non-sorted output of build. This makes every test provide coverage for how kustomize re-orders (or d... — committed to monopole/kustomize by monopole 5 years ago
Modify tests to present expected data in unsorted order. Modify all `build` tests to use the raw, non-sorted output of build. This makes every test provide coverage for how kustomize re-orders (or d... — committed to monopole/kustomize by monopole 5 years ago
Modify tests to present expected data in unsorted order. Modify all `build` tests to use the raw, non-sorted output of build. This makes every test provide coverage for how kustomize re-orders (or d... — committed to monopole/kustomize by monopole 5 years ago

Most upvoted comments

What if anything needs to be done here?

a user can define input order to match their desired output order and enter:
```
kustomize build --reorder none {target} | kubectl apply -f -
```
Regardless, ordering backend deployments before frontends in one apply isn’t sufficient, or even necessary, to bring up a healthy system.

background

kustomize build has a --reorder flag that accepts only two options, legacy and none.

The default value is legacy.
- This is a heuristic sort (e.g. emit Namespaces first, do WebHooks last).
- It keys off kind, and is controlled by the Gvk.IsLessThan function.
- This sort dates back to the early days when kustomize internals were map based, and didn’t respect input order. To get deterministic output, a sort was added.
- At some point (mid 2019?) kustomize changed to retain and respect input order. The sort just before output had to be retained as the default since people expected it.
- Perhaps we can change this default via popular demand?
Under none, the output order then matches the input order - first in, first out (FIFO).
- if resource R1 is specified before resource R2 in a kustomization file resources field, R1 will come out before R2. The kinds are irrelevant.
- Objects from the resources field will come out before objects from the generated field.
- The resources field can refer to other kustomizations, and these are recursively generated in the order specified by resources. So in the larger picture of a directed graph of overlay-bases, output order is a depth-first-traversal.
- All of the tests rely on this FIFO (non-legacy) behavior. To prove this, pick a test, e.g. namereferece_test, change the order in a resources field, run the tests, and see what happens.

That said, kubernetes is built on the notion of controllers being told to achieve some state, and then trying to get to that state in a non-centrally controlled fashion. Controllers can be expected to be interrupted in mid-change to go to some other state instead.

So, the API server, given an apply request with N resources, doesn’t perform a serial update - waiting for item 1 to finish updating, then kicking off 2, etc.

The cases where it appears to do so, e.g. namespace creation, are outliers.

monopole on Apr 13, 2021

This could be avoided when the original input source order would be respected as kubectl apply -f does it. For this reason I agree with @anguslees that the original order should be preserved.

I strongly agree with this. Baking in dependency re-ordering requires understanding all types.

liggitt on May 23, 2019

What was the downside to leaving ordering up to the user, based on the order of the manifest appeared in the kustomization.yaml resources array? What motivated explicit ordering in gvk.go?

brichins on May 28, 2019

I changed the order so that ValidatingWebhookConfiguration now comes last. This makes a cert-manager.yaml installation work with kustomize as well.

However unnecessary problems caused by ordering may still appear in theory when using objects with a CustomResourceDefinition’s kind where one has a hard dependency to the other. This could be avoided when the original input source order would be respected as kubectl apply -f does it. For this reason I agree with @anguslees that the original order should be preserved.

The question is: why was a kustomize-specific order introduced in the first place?

Edit: Also I didn’t touch the MutatingWebhookConfiguration order since I had no example to test at hand but I think it must come last as well.

mgoltzsche on May 23, 2019

Fwiw, I suggest replacing #822 with something that orders Webhook declarations last.

In a single invocation (and without more human-driven direction), we have to assume that we aren’t able to perform the webhook until after Deployments/etc have started, and we also have to assume that other resources in the same invocation don’t require the (mutating) webhook to act on them (that would be a bootstrapping cycle).

(Edit: or better yet for kustomize: preserve original document order)

anguslees on Mar 5, 2019

kustomize already sorts things like namespaces for this reason. CRDs and Webhooks should also be sorted IMHO.

pwittrock on Feb 25, 2019