Document the crossplane.io/external-create-... annotations #688

negz · 2024-01-30T03:19:01Z

These annotations were introduced in crossplane/crossplane-runtime#283.

Per crossplane/crossplane#3037 folks find these annotations hard to reason about. That's understandable, because they're doing a lot of subtle things.

This section ended up super long, but I think this is an area where folks really need to understand what's happening in order to make good decisions when Crossplane refuses to proceed.

These annotations were introduced in crossplane/crossplane-runtime#283. Per crossplane/crossplane#3037 folks find these annotations hard to reason about. That's understandable, because they're doing a lot of subtle things. This section ended up super long, but I think this is an area where folks really need to understand what's happening in order to make good decisions when Crossplane refuses to proceed. Signed-off-by: Nic Cope <nicc@rk0n.org>

netlify · 2024-01-30T03:19:06Z

✅ Deploy Preview for crossplane ready!

Name	Link
🔨 Latest commit	`0aef9a6`
🔍 Latest deploy log	https://app.netlify.com/sites/crossplane/deploys/65bab181d4fc2d0008fed4c6
😎 Deploy Preview	https://deploy-preview-688--crossplane.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 89 (no change from production) Accessibility: 88 (🔴 down 3 from production) Best Practices: 83 (no change from production) SEO: 93 (no change from production) PWA: 70 (no change from production) View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

plumbis

A few questions and some edits just to make some of the writing more direct. Nothing major. Great content!

content/master/concepts/managed-resources.md

negz · 2024-01-31T01:45:10Z

@plumbis PTAL. I started addressing feedback and ended up with a fairly extensive refactoring.

Signed-off-by: Nic Cope <nicc@rk0n.org>

plumbis · 2024-01-31T14:51:50Z

content/master/concepts/managed-resources.md

@@ -635,21 +635,145 @@ my-rds-instance      True    True     my-custom-name       11m

 ### Creation annotations

-Providers create new managed resources with the
-`crossplane.io/external-create-pending` annotation.
+In some rare situations a provider can forget that it created a resource. When


I don't like the use of "rare" (subjective) or "forget" (anthropomorphic) .

Rare sounds non-deterministic. It may require a specific sequence but the sequence is known and should be described. It may be rare in general but when you are hitting all the time hearing it's "rare" is very frustrating.

Point taken. Reworded to avoid "rare" and "forgot".

That said, I would prefer if folks did not read this section and think "Crossplane's going to leak my resources all the time, I shouldn't trust it".

content/master/concepts/managed-resources.md

plumbis · 2024-01-31T15:16:06Z

content/master/concepts/managed-resources.md

+If the provider can't find a managed resource in an external system, it thinks
+the resource doesn't exist. When the provider thinks a resource doesn't exist
+it creates the resource.


Suggested change

If the provider can't find a managed resource in an external system, it thinks

the resource doesn't exist. When the provider thinks a resource doesn't exist

it creates the resource.

If the provider finds a managed resource with a

{{<hover label="creation" line="7">}}crossplane.io/external-name{{</hover>}}

annotation and doesn't find an external resource with a matching name, the

provider creates a new external resource.

Feel free to rework, but I think this should be "dumbed down" a bit and be more explicit.

What is the provider looking for in the external system?

How does that get connected to an MR?

Avoid "think"

Reworked, PTAL.

I prefer not to mention external-name again explicitly here. It feels repetitive - how the provider looks up the MR - which is what the sentence immediately prior says.

"If the provider finds an MR with..." reads strangely to me too - MRs always have this annotation.

content/master/concepts/managed-resources.md

Signed-off-by: Nic Cope <nicc@rk0n.org>

luxas

Thanks a lot for this write-up @negz 💯

Level triggered systems are very interesting 😄

One of my favorite presentations about this is from thockin (avoiding at-ing him here): https://speakerdeck.com/thockin/kubernetes-what-is-reconciliation

With that comes to mind, that how many Crossplane controllers are able to attach (in general, approximately) labels, annotations, or similar to their managed data in the external provider, such that they can (at least in theory), find and point out all of these leaked resources, as they can see that they are indeed owned by "me" (through that label or annotation in the external system), but there is no external-name reference to them in Kube?

This indeed, would require controllers to have some kind of "identity", however, which is probably a big challenge itself (such that two controllers in two different clusters could distinguish their managed objects in the external system).

luxas · 2024-02-01T13:01:57Z

content/master/concepts/managed-resources.md

-`crossplane.io/external-create-failed` annotation after making the external API
-call and receiving a response. 
+{{<hint "tip">}}
+Crossplane calls resources that a provider creates but doesn't manage _leaked


Suggested change

Crossplane calls resources that a provider creates but doesn't manage _leaked

Crossplane calls resources that a provider created but lost track of and therefore doesn't manage _leaked

trying to word in such a way so that readers would understand instantly that this is an error case, without using the word "error", "exception" or "rare"

luxas · 2024-02-01T13:04:33Z

content/master/concepts/managed-resources.md

+
+When the external system generates the resource's name, the provider attempts to
+save it to the managed resource's `crossplane.io/external-name` annotation. If
+it doesn't, it _leaks_ the resource.


Suggested change

it doesn't, it _leaks_ the resource.

it cannot (after multiple retries), it _leaks_ the resource.

luxas · 2024-02-01T13:08:10Z

content/master/concepts/managed-resources.md

+Each time a provider reconciles a managed resource it checks the resource's
+creation annotations. If the provider sees a create pending time that's more
+recent than the most recent create succeeded or create failed time, it knows
+that it might have leaked a resource.


Are there any other common cases where this would happen than when someone went to the AWS UI and removed a reconciled bucket manually, and then Crossplane tried to recreate it, but failed to save the new name in the annotations?

No, nothing comes to mind.

negz · 2024-02-01T20:01:25Z

how many Crossplane controllers are able to attach (in general, approximately) labels, annotations, or similar to their managed data in the external provider, such that they can (at least in theory), find and point out all of these leaked resources, as they can see that they are indeed owned by "me" (through that label or annotation in the external system), but there is no external-name reference to them in Kube?

@luxas We considered this. It's a non-zero amount of controllers. However given:

The number of controllers (thousands)
The asymmetry of the APIs those controllers orchestrate (hundreds? thousands?)
That many of those APIs are eventually consistent (will take a while for reads to show results after a write)
The fact that many APIs that support tags/labels use a separate API call to add them (after create)

I'm skeptical there's value in exploring that path. Or at least it's deep into diminishing returns. At best we (Crossplane core) could offer some interface an ExternalClient (MR implementation) could optionally satisfy to lookup by label rather than by name and fall back to using that. It would then be up to any MR implementation that could to satisfy that interface (presumably by hand-writing the relevant code).

negz · 2024-02-01T20:06:54Z

Thanks folks! We've spent a lot of time this week wordsmithing one this week. I like where it's at right now and I need to move on to other things, so I'm going to merge it as-is. 🙂

This was referenced Jan 30, 2024

reconciler/managed: make more resilient to error conditions crossplane/crossplane-runtime#651

Merged

Reconcile blocked by crossplane.io/external-create-pending annotation crossplane/crossplane#3037

Closed

plumbis requested changes Jan 30, 2024

View reviewed changes

negz force-pushed the critical branch from 955c48e to 57362eb Compare January 31, 2024 01:44

negz requested a review from plumbis January 31, 2024 01:44

Refactor external creation annotations section

6cc91a7

Signed-off-by: Nic Cope <nicc@rk0n.org>

negz force-pushed the critical branch from 57362eb to 6cc91a7 Compare January 31, 2024 01:48

plumbis requested changes Jan 31, 2024

View reviewed changes

negz added 9 commits January 31, 2024 11:33

Be specific about circumstances where we'll leak

1341e35

Signed-off-by: Nic Cope <nicc@rk0n.org>

Use kubectl get -o yaml to get YAML manifest

741929c

Signed-off-by: Nic Cope <nicc@rk0n.org>

Use 'lookup' rather than 'find' an MR in an external system

9a5ee38

Signed-off-by: Nic Cope <nicc@rk0n.org>

Describe external name generation as nondeterministic, not unpredictable

26c4ff4

Signed-off-by: Nic Cope <nicc@rk0n.org>

Don't explicitly mention that saving annotations is critical

c0dbc0f

Signed-off-by: Nic Cope <nicc@rk0n.org>

Use if-then for inspecting external system

ecaf5c2

Signed-off-by: Nic Cope <nicc@rk0n.org>

Clarify that resuming reconciliation recreates MR

eafb527

Signed-off-by: Nic Cope <nicc@rk0n.org>

Move hint about why we wait for human intervention

c50047e

Signed-off-by: Nic Cope <nicc@rk0n.org>

Don't say "think", restructure important note about dupes

0aef9a6

Signed-off-by: Nic Cope <nicc@rk0n.org>

negz requested a review from plumbis January 31, 2024 21:04

luxas reviewed Feb 1, 2024

View reviewed changes

plumbis approved these changes Feb 1, 2024

View reviewed changes

negz merged commit 7a1ec9f into crossplane:master Feb 1, 2024
7 checks passed

negz deleted the critical branch February 1, 2024 20:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document the crossplane.io/external-create-... annotations #688

Document the crossplane.io/external-create-... annotations #688

negz commented Jan 30, 2024

netlify bot commented Jan 30, 2024 •

edited

Loading

plumbis left a comment

negz commented Jan 31, 2024

plumbis Jan 31, 2024

negz Jan 31, 2024

plumbis Jan 31, 2024

negz Jan 31, 2024

luxas left a comment

luxas Feb 1, 2024

luxas Feb 1, 2024

luxas Feb 1, 2024

luxas Feb 1, 2024

negz Feb 1, 2024

negz commented Feb 1, 2024 •

edited

Loading

negz commented Feb 1, 2024

-If the provider can't find a managed resource in an external system, it thinks
-the resource doesn't exist. When the provider thinks a resource doesn't exist
-it creates the resource.
+If the provider finds a managed resource with a
+{{<hover label="creation" line="7">}}crossplane.io/external-name{{</hover>}}
+annotation and doesn't find an external resource with a matching name, the
+provider creates a new external resource.

	Crossplane calls resources that a provider creates but doesn't manage _leaked
	Crossplane calls resources that a provider created but lost track of and therefore doesn't manage _leaked

	it doesn't, it _leaks_ the resource.
	it cannot (after multiple retries), it _leaks_ the resource.

Document the crossplane.io/external-create-... annotations #688

Document the crossplane.io/external-create-... annotations #688

Conversation

negz commented Jan 30, 2024

netlify bot commented Jan 30, 2024 • edited Loading

✅ Deploy Preview for crossplane ready!

plumbis left a comment

Choose a reason for hiding this comment

negz commented Jan 31, 2024

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

luxas 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

negz commented Feb 1, 2024 • edited Loading

negz commented Feb 1, 2024

netlify bot commented Jan 30, 2024 •

edited

Loading

negz commented Feb 1, 2024 •

edited

Loading