feat(design): project and resource simulate#181
Conversation
Signed-off-by: Christopher Haar <christopher.haar@upbound.io>
Signed-off-by: Christopher Haar <christopher.haar@upbound.io>
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
We demonstrated the end to end flow, the CLI driving provider plan servers against a live control plane, in the Crossplane community meeting: see the |
| preview of the revision advance rather than of the apply, and counted | ||
| separately in the summary. crossplane-diff reached the same conclusion from | ||
| the other direction: its composition diffing excludes Manual-policy | ||
| composites by default behind an `--include-manual` flag. This keeps that |
There was a problem hiding this comment.
is there a reason why we wouldn't keep the same nomenclature?
I went with manual for the flag because Manual is the enumeration value for the compositionUpdatePolicy.
| 1. **Discover.** Parse the project file, find the Compositions under the | ||
| project's API paths, and list the XRs on the target cluster whose type | ||
| each Composition composes. `--namespace` and `--name` scope the run. | ||
| 2. **Prepare.** Build the project's embedded functions locally, exactly as |
There was a problem hiding this comment.
I think we should have a variant of this that pulls these from the live cluster (at least, any not overridden locally) -- unless you mean to scope resource simulate to include -f composition.yaml as well?
I don't think everyone is using projects (yet?) and the folks who aren't will likely still want to diff a composition.
| review. The existing maintainers stay code owners of what they built, the | ||
| standalone repo enters maintenance with a pointer forward, and every | ||
| Crossplane user gets the preview by default: one binary, one release | ||
| train, one CI matrix. |
There was a problem hiding this comment.
one thing i am attached to that saves a bunch of time in diff development is the integration test suite with setup-envtest.
it's about 7 seconds to run a full IT through kong input -> machine output against a sort-of-fake cluster, versus minutes to spin up the e2e kind cluster. because it's so (relatively) quick, it covers a lot of surface area and will be valuable for keeping regressions to a minimum during the transition.
we'll probably want to port that over for any CLI commands that benefit from testing against a cluster. it could form a foundation of the e2e/IT testing work we've been meaning to tackle.
| transplant of the repo, but a joint pass with its maintainers over what | ||
| simulate and a first-class diff experience actually need, moving those | ||
| pieces into crossplane/cli as shared internals, and leaving behind what the | ||
| standalone packaging forced them to build. |
There was a problem hiding this comment.
I agree with this. it seems pretty core to the crossplane experience. in fact my initial diff proposal was a PR to crossplane/crossplane.
| fetches what functions ask for from the live cluster), the diff | ||
| calculator (rendered against live, including removal detection and nested | ||
| XRs), the server-side dry-run that keeps defaulting and webhook mutation | ||
| out of the diff, and the composition impact discovery behind |
There was a problem hiding this comment.
these already build on c/cli: the impact discovery uses the xrmClient of crossplane trace. and we run all the XRs through crossplane validate.
this will probably mean promoting trace and validate to GA (or at least the libraries they build on)
| and a first-class diff command share. | ||
| * **What gets simpler in the move.** Living outside the CLI forced | ||
| indirection the code no longer needs once it lives inside. The render | ||
| seam it uses to drive the CLI's engine from another binary collapses into |
There was a problem hiding this comment.
the seam isn't really going to collapse. the real seam is the docker seam between internal render and the CLI itself. we mostly just call the methods that are provided.
| * **What gets simpler in the move.** Living outside the CLI forced | ||
| indirection the code no longer needs once it lives inside. The render | ||
| seam it uses to drive the CLI's engine from another binary collapses into | ||
| a direct call, and its loading and validation glue merges with the CLI's |
There was a problem hiding this comment.
i've polished up the loading and validation logic a little bit in the process of building diff, and submitted the improvements upstream.
| rendered counterpart means the change would remove it, and it enters the | ||
| plan as a delete instead of silently disappearing from the preview. | ||
|
|
||
| Printing then follows one rule: show users their own manifest, annotated. |
There was a problem hiding this comment.
it will be a conscious choice to switch from kubectl diff style syntax to terraform plan syntax.
maybe we should see which way consumers would prefer.
since my use case is largely for CI, i tried to build diff to use an output format that would be compatible with markdown's diff interpreter syntax for code blocks, so line by line replacement:
- foo: bar
+ foo: baz
| (@tampakrap), Adam Wolfe-Gordon (@adamwg) | ||
| * Status: Draft | ||
|
|
||
| ## Background |
There was a problem hiding this comment.
one thing i don't see mentioned here is claims. we have a lot of people still on crossplane v1's object model; any specific goal or non-goal about those?
diff does try pretty hard to work against legacy objects.
| discovery. Both commands enter the tree with the CLI's `maturity:"alpha"` | ||
| tag and graduate the way every other command does. | ||
|
|
||
| ### Running Plan Servers Like Render Runs Functions |
There was a problem hiding this comment.
one thing i found out the hard way is that diff is slow unless you keep containers alive across runs. every run might stand up a crossplane container, all function containers, now provider containers, and then tear them down. we should explicitly account for container orchestration: strong names, maybe even a top level --orphan-containers flag in case you need to run some things serially.
| render and crossplane-diff already share. Functions that require extra | ||
| resources get them the way crossplane-diff resolves them today: | ||
| iteratively, fetching what the pipeline requests from the live cluster | ||
| and re-rendering until requirements stabilize, with an iteration cap. |
There was a problem hiding this comment.
"iteration cap" reminds me of failure modes. diff is designed to fail fast and not emit partial results (for a given resource's tree) to avoid giving a false sense of confidence. is that a design principle we want to adhere to, here, or do we think "best effort" instead?
we should also agree on an error contract for structured mode. diff writes both stderr and the structured payload OutputError but that's a relatively new change for structured mode.
| The CLI treats provider packages the way render treats function packages: as | ||
| images it can run locally and talk to over gRPC. | ||
|
|
||
| From the project's `dependsOn` (or an explicit `--provider-images` override, |
There was a problem hiding this comment.
how does a provider-images override work? how do i know which provider serves a given api group without a project? are we looking into the cluster for this (it's what diff would do)? and if we're querying the cluster anyway, shouldn't we just query it and keep the image override optional?
(i suppose we could also query the provider registry, but that seems like an added dependency we don't want)
2 participants
Description of your changes
This proposes
crossplane project simulateandcrossplane resource simulate: a terraform plan style preview of what a Composition, embedded function, XR, MR, or provider version change would do, both to the cluster and to the cloud behind it.Rendering and cluster diffing build on crossplane-contrib/crossplane-diff rather than reimplementing it, and the proposal is explicit that converging that work into the CLI happens together with its maintainers, who are proposed reviewers here. The cloud layer drives per-provider plan servers over gRPC, the way render drives function runtimes.
Companion proposal in crossplane/upjet#694 defines the PlanService protocol and the provider-side implementation.
Fixes #
I have:
./nix.sh flake checkto ensure this PR is ready for review.backport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.