kubectl apply reads a file or set of files, and updates the cluster state based off the file contents.
It does a couple things:
Essential complexity in the apply code comes from supporting custom strategies for
merging fields built into the object schema -
such as merging lists together based on a field
key and deleting individual
items from a list by
Accidental complexity in the apply code comes from the structure growing organically in ways that have
broken encapsulation and separation of concerns. This has lead to maintenance challenges as
keeping ordering for items in a list, and correctly merging lists of primitives (
Round tripping changes through PATCHes introduces additional accidental complexity, as they require imperative directives that are not part of the object schema.
Reduce maintenance burden by minimizing accidental complexity in the apply codebase.
This should help:
Implementation of proposed changes under review in PR 52349
Building a PATCH from diff creates additional code complexity vs directly updating the object.
In the current implementation of apply - parsing and traversing the object trees, diffing the contents and generating the patch are entangled. This creates maintenance and testing challenges. We should instead encapsulate discrete responsibilities in separate packages - such as collating the object values and updating the target object.
Provide a structure that contains the last, local and live value for each field. This will make it easy to walk the a single tree when making decisions about how to update the object. Decisions about ordering of lists or parsing metadata for fields are made here.
Use the visitor pattern to encapsulate how to update each field type for each merge strategy. Unit test each visit function. Decisions about how to replace, merge, or delete a field or list item are made here.
In order to make apply sufficiently maintainable and extensible to new API types, as well as to make its behavior more intuitive for users, the merge behavior, including how it is specified in the API schema, must be systematically redesigned and more thoroughly tested.
Examples of issues that need to be resolved
mergeKeyare implicit, unversioned and incorrect in some cases. to fix the incorrect metadata, the metadata must be versioned so PATCHes generated will old metadata continue to be merged by the server in the manner they were intended
The following PRs constitute the focus of ~6 months of engineering work. Each of the PRs is very complex for the work what it is solving.
38665 - Support deletion of primitives from lists - Lines (non-test): ~200 - ~6 weeks 44597 - Support deleting fields not listed in the patch - Lines (non-test): ~250 - ~6 weeks 45980 - Keep ordering of items when merging lists - Lines (non-test): ~650 46161 - Support using multiple fields for a merge key - Status: Deferred indefinitely - too hard for maintainers to understand impact and correctness of changes 46560 - Support diff apply (1st attempt) - Status: Closed - too hard for maintainers to understand impact and correctness of changes 49174 - Support diff apply (2nd attempt) - Status: Deferred indefinitely - too hard for maintainers to understand impact and correctness of changes - Maintainer reviews: 3
Apply is implemented by diffing the 3 sources (last-applied, local, remote) as 2 2-way diffs and then merging the results of those 2 diffs into a 3rd result. The diffs can each produce patch request where a single logic update (e.g. remove ‘foo’ and add ‘bar’ to a field that is a list) may require spreading the patch result across multiple pieces of the patch (a ‘delete’ directive, an ‘order’ directive and the list itself).
Because of the way diff is implemented with 2-way diffs, a simple bit of logic “compare local to remote” and do X - is non-trivial to define. The code that compares local to remote is also executed to compare last-applied to local, but with the local argument differing in location. To compare local to remote means understanding what will happen when the same code is executed comparing last-applied to local, and then putting in the appropriate guards to short-circuit the logic in one context or the other as needed. last-applied and remote are not compared directly, and instead are only compared indirectly when the 2 diff results are merged. Information that is redundant or should be checked for consistency across all 3 sources (e.g. checking for conflicts) is spread across 3 logic locations - the first 2-way diff, the second 2-way diff and the merge of the 2 diffs.
That the diffs each may produce multiple patch directives + results that constitute an update to a single field compounds the complexity of that comparing a single field occurs across 3 locations.
The diff / patch logic itself does not follow any sort of structure to encapsulate complexity into components so that logic doesn’t bleed cross concerns. The logic to collate the last-applied, local and remote field values, the logic to diff the field values and the logic to create the patch is all combined in the same group of package-scoped functions, instead of encapsulating each of these responsibilities in its own interface.
Sprinkling the implementation across dozens of locations makes it very challenging to flag guard the new behavior. If issues are discovered during the stabilization period we cannot easily revert to the previous behavior by changing a default flag value. The inability to build in these sorts of break-glass options further degrades confidence in safely accepting PRs.
This is a text-book example of what the Visitor pattern was designed to address.
If the apply diff logic was redesigned, most of the preceding PRs could be implemented by only touching a few existing code locations to introduce the new type / method, and then encapsulating the logic in a single type. This would make it simple to flag guard new behaviors before defaulting them to on.