Kustomize Resource Ordering Proposal #865

pwittrock · 2019-03-01T19:23:17Z

Proposal to address

kubernetes-sigs/kustomize#836

k8s-ci-robot · 2019-03-01T19:23:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pwittrock

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:

~~keps/sig-cli/OWNERS~~ [pwittrock]

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

pwittrock · 2019-03-01T19:23:49Z

/assign @monopole

pwittrock · 2019-03-01T19:28:52Z

cc @donbowman

apelisse · 2019-03-01T20:01:13Z

keps/sig-cli/kustomize-ordering.md

+
+Kustomize orders Resource creation by sorting the Resources it emits based off their type.  While
+this works well for most types (e.g. create Namespaces before other things), it doesn't work
+in all cases and users will need the ability to break glass.


I can guess why, but it's not obvious when this ordering becomes problematic and when you can't rely on type ordering anymore (or at least an example would be useful)

Did you look at the example in the motivation?

You mean in the body of the PR, not in that KEP right?

Can you replace "break glass" with a more direct but short mention of using labels and annotaitons?

Also, can we just get by with one or the other at the outset? I.e. ship label-based ordering then add annos if someone asks for it (or vice versa)

Also, can we just get by with one or the other at the outset? I.e. ship label-based ordering then add annos if someone asks for it (or vice versa)

I like this idea, I think it's fine to be opinionated. You could almost enforce a specific annotation and have kustomize remove it. Giving people the chance to use any label/annotation is maybe not restrictive enough, as they will be able to build arbitrary complex things with a poor semantic (noted in "risk & mitigation", I know), but I think we could help them by making a reasonable decision on what annotations to use (e.g. I know you won't like it: kustomize.io-order-priority: X).

Alternative solutions:
I don't know if the order should be in each object (more implicit, and less coupled) or in the kustomize file directly (stronger coupling, more explicit).
You could also build something like an "edge"/graph based solution, e.g. this object before that one, or this after that.

apelisse · 2019-03-01T20:55:13Z

keps/sig-cli/kustomize-ordering.md

+
+### Goals
+
+- Provide the ability for users to break glass and override Kustomize's sort ordering.


See, I didn't even realized that kubectl had to apply the resources in the order they are received. Kubectl could also parallelize the requests to make order "less important". A lot of this is still going to be flaky because it has to assume that the resources will be created "within time-limit" (not just in-order), since we're talking about an asynchronous system here.

Resource Creation and Resource Actuation are different things. This is concerned only with the former, which will be serial.

donbowman · 2019-03-01T22:16:35Z

OK so in my original example the 'order dependent' bits, and their needed order, are:

Namespace
Namespace label
ValidatingWebhookConfiguration
CRD

So with the above proposal, I can achieve this by adding these lines to the base kustomization.yaml:

resourceOrdering:
- type: label
  key: MyBeautifulOrder
  value: 1
- type: label
  key: MyBeautifulOrder
  value: 2
- type: label
  key: MyBeautifulOrder
  value: 3

And then in my config I would have:

---
apiVersion: v1
kind: Namespace
metadata:
  name: dev-fluent-bit
  labels:
    mylabel: value
    MyBeautifulOrder: 1
---
apiVersion: v1
kind: ValidatingWebhookConfiguration
metadata:
  name: MyVWC
  labels:
    MyBeautifulOrder: 2
---
apiVersion: v2
kind: CustomResourceDefinition
metadata:
  name: MyCRD
  labels:
    MyBeautifulOrder: 3

So...

Do i think it fixes? Yes.
Is it relatively idiot proof? Yes
Is it relatively convenient to do? Yes. Its a bit of a PITA since i end up with a label in the 'runtime' that really only relates to the 'build time', but I can live w/ that.

It leaves the question of... when i'm doing a delete, I may want the opposite order, would we have a --reverse flag to specify reversing the order?

apelisse · 2019-03-01T22:18:32Z

It leaves the question of... when i'm doing a delete, I may want the opposite order, would we have a --reverse flag to specify reversing the order?

If you're running kubectl delete -k ..., then it could automatically delete in the reverse order?

pwittrock · 2019-03-01T22:33:16Z

@donbowman There are a couple of ideas being kicked around for delete. I think this will align with at least one of them - using annotations.

One of the deletion proposals is to leave the resource and annotate with something special like kubectl.k8s.io/delete=true. The approach here would allow you to control the deletion order using the same mechanism.

monopole

Thanks!

monopole · 2019-03-02T01:16:29Z

keps/sig-cli/kustomize-ordering.md

+
+## Docs
+
+Update Kubectl Book and Kustomize Documentation


monopole · 2019-03-02T01:17:41Z

keps/sig-cli/kustomize-ordering.md

+- [ ] Having multiple labels / annotations with different keys and the same values works correctly
+- [ ] Mixing labels and annotations works
+- [ ] Unrecognized type values throw and error
+- [ ] Resources that don't appear in the ordering overrides appear last and are sorted by type


Well, that will be one of the most thoroughly tested features we have, thanks!

The place to most of these is perhaps in pkg/target (see any test file in there).

monopole · 2019-03-02T01:19:28Z

keps/sig-cli/kustomize-ordering.md

+
+Kustomize orders Resource creation by sorting the Resources it emits based off their type.  While
+this works well for most types (e.g. create Namespaces before other things), it doesn't work
+in all cases and users will need the ability to break glass.


Can you replace "break glass" with a more direct but short mention of using labels and annotaitons?

Also, can we just get by with one or the other at the outset? I.e. ship label-based ordering then add annos if someone asks for it (or vice versa)

pwittrock · 2019-03-03T17:37:10Z

Also, can we just get by with one or the other at the outset? I.e. ship label-based ordering then add annos if someone asks for it (or vice versa)

Yes I think so. Probably start with annotations as we have been discussing using them to configure kubectl apply behavior (e.g. deletions, cluster targeting, etc)

pwittrock · 2019-03-04T04:24:54Z

Simplified the API a bit. PTAL.

bgrant0607 · 2019-03-04T23:13:18Z

keps/sig-cli/kustomize-ordering.md

+
+## Motivation
+
+Users may need direct control of the Resource create / update / delete ordering in cases were


The ordering for each of those operations may not always be the same

anguslees · 2019-03-05T11:48:35Z

keps/sig-cli/kustomize-ordering.md

+
+## Implementation History
+
+## Alternatives


Please include a discussion of at least some alternatives, otherwise what's the point of the KEP?
(If there is only one possible option, then there is no need for a KEP doc. If there are alternatives, then better to have the rationale for selecting this particular approach expounded by the original authors)

In particular for this doc: The obvious alternative that I think needs to be considered is just process the resources in original document order. This is presumably the order intended by the upstream manifest author (since this is kubectl's behaviour). We would need a bit of explanation for the kustomize corner cases like newly-inserted resources, and when multiple upstreams are merged (conceivably any order that preserves the original separtae document order(s) is fine here).

Please include a discussion of at least some alternatives, otherwise what's the point of the KEP?

To get other community members to come up with alternatives :)

The obvious alternative that I think needs to be considered is just process the resources in original document order

Would this be mutually exclusive with the sorted order, so users would need to manually put CRDs first, etc?

Would this be mutually exclusive with the sorted order

I don't understand... Obviously you can't primary-sort by both document order and some other (eg: type-based) order.

users would need to manually put CRDs first, etc?

Yes, "document order" assumes that the user (or some other tool) has appropriately sorted the resources in the input document. Again: kubectl works in document order, so this is already quite a common workaround/technique.

anguslees · 2019-03-05T12:07:17Z

Fwiw, the create/update sort order in kubecfg is:

CRD/TPR first (special case)
all non-namespaced resources
all resources that don't fit somewhere else
all resources that contain a PodSpec
(to be done) webhooks last (special case)

'delete' reverses that order. In practice there's a secondary alphabetic sort within those tiers, just for output stability.

That's obviously not a "break glass" approach, but it has worked very well so far and seems to have been more robust than the hard-coded lists used by helm and kustomize. Just mentioning it here, because I think this is the only reflection-based sort heuristic I've seen in use in k8s tools.

pwittrock · 2019-03-07T16:53:33Z

@anguslees Would the order you described address the issue linked in the PR?

pwittrock · 2019-03-07T16:58:18Z

'delete' reverses that order.

How does kubecfg perform deletions?

Liujingfang1 · 2019-03-11T18:03:33Z

keps/sig-cli/kustomize-ordering.md

+Provide a simple mechanism allowing users to override the order that
+Resource operations are Applied.  Add a new field `sortOrder` that is
+a list of `ResourceSelector`s which match Resources based
+off their annotations.


Within one kustomization target, I think this solves the ordering problem. I'd like to see explanations or examples covering bases and overlays. Given an overlay composed from multiple bases. Does the order from each base get inherited in overlays? What if I want resources from one base to be prior to resources from another base?

Good point, what about conflicting orderings (cycles)

Liujingfang1 · 2019-03-11T18:09:48Z

keps/sig-cli/kustomize-ordering.md

+everything that the second selector does, and it will have no effect.
+
+```
+sortOrder:


How about generating the output in the same order as they are listed in resources. If one resource file containing multiple resource configs, keep their original order? Moreover, keep the order of bases.
In Kustomization.yaml, we can add one field to switch to this behavior:

keepOriginalOrder: true

anguslees · 2019-03-12T00:33:04Z

(I don't want this to be about kubecfg, and I'm happy to follow this up in some other forum)

@anguslees Would the order you described address the issue linked in the PR?

Yes, I think so - although the bug is a bit difficult to dissect and contains a number of inaccurate/overspecified requirements (iiuc).

I think the original issue is that (after kustomize re-sorting) the ValidatingWebhook is declared before the Deployment, etc that implements the Webhook. Then a resource is created that triggers the Webhook, and the apiserver rejects the creation, because the webhook isn't ready yet (and the WebhookConfiguration has failurePolicy=Fail).

There are numerous changes that would satisfy the original issue:

Sort WebhookConfiguration declarations after Deployment (ie: the opposite of Capture and iterate on KEP template feedback #822). The "kubecfg order" I described above would also do this.
Preserve original document order
Change cert-manager upstream to use failurePolicy=Ignore

How does kubecfg perform deletions?

It has a garbage collection mechanism that determines what is eligible for deletion. To actually perform the deletions, it just sorts in the reverse creation order, and calls client-go Delete() on each in turn.

pwittrock · 2019-03-12T15:51:40Z

@anguslees

Thanks for the comments. Not sure what the best next steps are. IMHO, GC is more urgent and critical than this issue. I opened the KEP because a user mentioned it as a problem, but it can be worked around by creating separate kustomization.yamls and applying them separately.

Trying to solve whether or not one resource is ready before another does not seem like a problem that should be solved at this layer (i.e. in kubectl or kustomize)

anguslees · 2019-03-13T04:01:42Z

... but it can be worked around by creating separate kustomization.yamls and applying them separately.
Trying to solve whether or not one resource is ready before another does not seem like a problem that should be solved at this layer (i.e. in kubectl or kustomize)

I'm still confused why we haven't discussed "preserve document order" anywhere here (with some hand-waving around resources that are newly introduced by kustomize (configmap/secret generators)).

At the moment, applying an empty kustomize file to an existing kubectl input file makes things worse, and that seems like something we could at least fix, even if we want to punt on the actually-hard cases for now.

pwittrock · 2019-03-14T18:30:04Z

I'm still confused why we haven't discussed "preserve document order" anywhere here

Generally I have concerns about this solution. A simple refactoring of configs, or sorting the resources alphabetically would break this. I don't think this is a solution we would want to recommend, but might be ok as a break glass feature for users that are ok with the risks it introduces.

apelisse · 2019-03-14T18:39:03Z

Generally I have concerns about this solution. A simple refactoring of configs, or sorting the resources alphabetically would break this. I don't think this is a solution we would want to recommend, but might be ok as a break glass feature for users that are ok with the risks it introduces.

Agreed, it's very implicit and fragile.

pwittrock · 2019-03-14T20:09:03Z

@anguslees @donbowman

I'd like to see someone from the community drive this (and hopefully implement it). I think we have a good conversation started here for someone from the community to pick up and incorporate the feedback. Plan on closing this to make room for others to step up.

pwittrock · 2019-03-14T20:09:32Z

Closing so that someone else can take lead on this.

anguslees · 2019-03-18T01:40:09Z

Agreed, it's very implicit and fragile.

Sure, I'm not claiming it's great and that we can't do better. This (document order) is the way kubectl apply (and other verbs) works, the community has built up around this assumption, and things like the original motivating bug here rely on this. It just seemed super-odd to have a sig-cli KEP that talked about resource ordering that didn't acknowledge/consider the existing sig-cli approach 😕

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 1, 2019

k8s-ci-robot requested review from seans3 and soltysh March 1, 2019 19:23

k8s-ci-robot assigned anguslees and monopole Mar 1, 2019

pwittrock requested review from apelisse and anguslees and removed request for seans3 March 1, 2019 19:24

pwittrock force-pushed the kustomize-order branch from 88fba5d to d1eadcf Compare March 1, 2019 19:24

pwittrock mentioned this pull request Mar 1, 2019

Issue #821 was closed without solving the issue kubernetes-sigs/kustomize#836

Closed

pwittrock force-pushed the kustomize-order branch from d1eadcf to e7f891b Compare March 1, 2019 19:25

pwittrock added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Mar 1, 2019

pwittrock unassigned anguslees Mar 1, 2019

pwittrock removed the request for review from soltysh March 1, 2019 19:26

pwittrock force-pushed the kustomize-order branch from e7f891b to 0530fc2 Compare March 1, 2019 19:26

apelisse reviewed Mar 1, 2019

View reviewed changes

Kustomize Resource Ordering Proposal

1df8b3e

pwittrock force-pushed the kustomize-order branch from 0530fc2 to 1df8b3e Compare March 1, 2019 22:36

monopole reviewed Mar 2, 2019

View reviewed changes

pwittrock force-pushed the kustomize-order branch from 0f788af to d2238c5 Compare March 4, 2019 04:24

pwittrock force-pushed the kustomize-order branch from d2238c5 to ae05bfc Compare March 4, 2019 04:26

update with pr comments

53ec758

pwittrock force-pushed the kustomize-order branch from ae05bfc to 53ec758 Compare March 4, 2019 16:53

bgrant0607 reviewed Mar 4, 2019

View reviewed changes

monopole mentioned this pull request Mar 5, 2019

kustomize re-orders config causing fail when crd or admission webhook used kubernetes-sigs/kustomize#821

Closed

anguslees reviewed Mar 5, 2019

View reviewed changes

Liujingfang1 reviewed Mar 11, 2019

View reviewed changes

pwittrock closed this Mar 14, 2019

kkasravi mentioned this pull request Jun 6, 2019

ISTIO related deployments are deployed to wrong namespace kubeflow/kubeflow#3375

Merged

4 tasks


		### Goals

		- Provide the ability for users to break glass and override Kustomize's sort ordering.


		## Motivation

		Users may need direct control of the Resource create / update / delete ordering in cases were

Kustomize Resource Ordering Proposal #865

Kustomize Resource Ordering Proposal #865

Conversation

pwittrock commented Mar 1, 2019 • edited Loading

k8s-ci-robot commented Mar 1, 2019

pwittrock commented Mar 1, 2019 • edited Loading

pwittrock commented Mar 1, 2019

apelisse Mar 1, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pwittrock Mar 1, 2019 • edited Loading

Choose a reason for hiding this comment

donbowman commented Mar 1, 2019

apelisse commented Mar 1, 2019

pwittrock commented Mar 1, 2019

monopole left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pwittrock commented Mar 3, 2019

pwittrock commented Mar 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anguslees commented Mar 5, 2019 • edited Loading

pwittrock commented Mar 7, 2019

pwittrock commented Mar 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anguslees commented Mar 12, 2019 • edited Loading

pwittrock commented Mar 12, 2019

anguslees commented Mar 13, 2019 • edited Loading

pwittrock commented Mar 14, 2019

apelisse commented Mar 14, 2019

pwittrock commented Mar 14, 2019

pwittrock commented Mar 14, 2019

anguslees commented Mar 18, 2019

pwittrock commented Mar 1, 2019 •

edited

Loading

pwittrock commented Mar 1, 2019 •

edited

Loading

apelisse Mar 1, 2019 •

edited

Loading

pwittrock Mar 1, 2019 •

edited

Loading

anguslees commented Mar 5, 2019 •

edited

Loading

anguslees commented Mar 12, 2019 •

edited

Loading

anguslees commented Mar 13, 2019 •

edited

Loading