crossplane-runtime icon indicating copy to clipboard operation
crossplane-runtime copied to clipboard

Managed Reconciler: ExternalObservation type should pass more information to the Create method

Open soorena776 opened this issue 4 years ago • 5 comments

What problem are you facing?

When using the managed reconciler pattern, it's sometimes the case that more than just one operation need to be performed in the external resource's Create method, conditioned on the observation made. For instance, an external resource might need 3 distinct sub-components to be provisioned in order, for it to be created. Currently, every time the Create method is called, it uses the CR's fields, to determine what operation it should run, which is not efficient and essentially the observation is done again in the Create method (it is triggered merely based on the fact that ExternalObservation{ ResourceExists: false }).

How could Crossplane help solve your problem?

More information (for instance a map of key-values) could be passed to Create method, making it more context-aware.

soorena776 avatar Mar 09 '20 16:03 soorena776

I'm not sure if I understood you correctly but in API patterns one-pager, we recommend that the one managed resource should result in one external resource, if that's what you mean. This is because ExternalClient interface is supposed to be the lowest level of operations in Crossplane, i.e. talk to the cloud provider. You can see the details here (section For both Status and Spec:).

About carrying a map; the assumption of ExternalClient is that all information necessary to provision the resource exists on the spec. Hence, in most of the cases Create just converts spec.forProvider into the cloud provider struct and call the provider's API.

Could you elaborate on what you're trying to achieve?

muvaf avatar Mar 10 '20 09:03 muvaf

@muvaf thanks for the comment!

My use case is that in order to provision an external resource A, the creation goes through a series of steps in order. Trying to keep it simple, let's say A roughly represents a KubernetesApplication on a KubernetesCluster. The Create method for A then will look like following:

  • It needs an instance of KubernetesCluster claim, and should wait for it to be satisfied (either finding an existing cluster, or provisioning a new one).
  • It then creates the specified KubernetesApplication on the created cluster.

As you can see, the Observe method has more than just a boolean ExternalResource information, and every time that the Create is called, it needs to know what step it had has to start from as there are more than one just a single action depending on what the observation was. One solution is to 'observe' the current step, but that goes against the purpose of Observe method. The other option would be to add some metadata to the status, or spec of the CR, but this metadata is only used for internal wiring and I don't see why it should be exposed.

If the assumption is that managed resource always results in creating only one external resource in a single phase, then the managed reconciler doesn't seem to be appropriate choice for my scenario, which is fine. However I won't be able to use the goodies that are provided by the crossplane-runtime library.

soorena776 avatar Mar 21 '20 19:03 soorena776

The closest case that I can see in Crossplane is route table resource on aws provider. A RouteTable resource needs multiple resources and even though all of them are created regardless of their existence in this case, it could be the case that the creation steps are not idempotent and only should run conditionally.

soorena776 avatar Mar 21 '20 19:03 soorena776

If the assumption is that managed resource always results in creating only one external resource in a single phase, then the managed reconciler doesn't seem to be appropriate choice for my scenario, which is fine.

This is the case ideally but I can imagine we do make some compromises as APIs are scattered with more than resources which need a Create call for each. So, it's not the ideal case we're aiming for but a case we have to consider.

That said, ExternalClient functions should be idempotent. What I'd do is to check everything you'd like to be created in Observe and if any one of them doesn't exist, return ResourceExists: false. Then make Create that has multiple calls idempotent; either skip the err if it's AlreadyExists or make GET request for each of them before creation. In case create call results in an identifier each time, you'd have to save that in spec (or in annotations as external name if it's the main resource's identifier), then check its existence.

The closest case that I can see in Crossplane is route table resource on aws provider. A RouteTable resource needs multiple resources and even though all of them are created regardless of their existence in this case, it could be the case that the creation steps are not idempotent and only should run conditionally.

Sadly, this seems like a buggy implementation where Create never called again even though createRoutes or createAssociations fail because in Observe, only existence of RouteTable is checked.

muvaf avatar Mar 23 '20 09:03 muvaf

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Aug 13 '22 07:08 stale[bot]

Crossplane does not currently have enough maintainers to address every issue and pull request. This issue has been automatically marked as stale because it has had no activity in the last 90 days. It will be closed in 14 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.

github-actions[bot] avatar Sep 05 '24 01:09 github-actions[bot]