[go: nahoru, domu]

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
soorena776 opened this issue Mar 9, 2020 · 5 comments
Labels
enhancement New feature or request wontfix This will not be worked on

Comments

@soorena776
Copy link
Contributor

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 soorena776 added the enhancement New feature or request label Mar 9, 2020
@muvaf
Copy link
Member
muvaf commented Mar 10, 2020

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?

@soorena776
Copy link
Contributor Author

@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
Copy link
Contributor Author
soorena776 commented Mar 21, 2020

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.

@muvaf
Copy link
Member
muvaf commented Mar 23, 2020

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.

@stale
Copy link
stale bot commented Aug 13, 2022

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 stale bot added the wontfix This will not be worked on label Aug 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants