This commit moves where we set the finalizer for managed resources to right
before creating them, not at the beginning of the reconcile. This means we'll be
less likely to encounter issues where we can't delete a managed resource because
we could never create it in the first place, but we added a finalizer.
* By the time we get here we know our Observe call worked. If (for example) our
cloud provider credentials were completely wrong, we'd never proceed far
enough to add the finalizer.
* If Observe works but Create fails (for example because we had RO cloud
provider credentials) we would already have added the finalizer, but...
* When the managed resource was deleted we'd be able to Observe that the
external resource does not exist (because we were never able to Create it) and
thus would not call Delete on the external resource and go straight to
unpublishing credentials and removing the finalizer.
This commit also renames and refactors a bunch of our interfaces to use less
obtuse names. Previously sometimes a "finalize" method unbound a managed
resource, while at other times it removed the finalizer. Similarly, finalizers
were added in "initialize". We now have a 'Binder' interface with bind and
unbind methods, and two 'Finalizer' interfaces (one for Claim, and one for
Managed) that add and remove finalizers, as you would expect.
Signed-off-by: Nic Cope <negz@rk0n.org>
This status update is only useful when:
* Some other controller also added a finalizer to this managed resource.
* The other controller has not yet processed the delete and removed its finalizer.
Given that this is a rare (or non-existent) edge case, and given that attempting
to update the status of a non-existent managed resource results in spurious
errors in the logs, I think it's safe to remove this logic.
Signed-off-by: Nic Cope <negz@rk0n.org>
Reference resolution is now a no-op if nothing changes, so we run it on every
reconcile. We also run it after delete has been handled, so unresolved
references will only block creates and updates.
This commit means we'll make more get calls to the cache (or API) in order to
resolve our references each reconcile, and also risk potentially changing the
values of 'immutable' fields automatically if and when our references resources
change. I believe we should address this by having referencers be no-ops when
the field value they would set is already set.
I attempted to move reference resolution to right before we call create or
update (i.e. after observe and delete), but it turns out certain resources
(specifically GCP Connections) could need references to be resolved in order to
observe the external resource.
Signed-off-by: Nic Cope <negz@rk0n.org>
The mock binding status was identical to the real one, while the mock
conditioned status set only the most recent condition, leading to a few
slightly broken managed resource reconciler tests.
Signed-off-by: Nic Cope <negz@rk0n.org>
the established convention is for the managed resource reconciler to requeue
after a short wait (typically 30 seconds) when it knows it is waiting for an
operation.
Signed-off-by: Nic Cope <negz@rk0n.org>
No functional updates to the code here, just tidying up the tests a bit to
ensure they all mock out their various moving parts via options.
Signed-off-by: Nic Cope <negz@rk0n.org>
- Removing the panic recovery logic
- Removing the superfluous IsConditionReady method
- Adding Nic's unit-tests for GetCondition and IsConditionTrue
- Minor other fixes
Signed-off-by: soorena776 <javad@upbound.io>
* - Introduces Initializer hook to managed reconciler.
- ManagedNameAsExternalName is introduced and used
by default by all managed reconcilers.
Signed-off-by: Muvaffak Onus <onus.muvaffak@gmail.com>
* GetExternalName and SetExternalName functions are implemented
Signed-off-by: Muvaffak Onus <onus.muvaffak@gmail.com>
* - Unit tests for ManagedNameAsExternalName struct
- Move ExternalNameAnnotationKey into meta package.
Signed-off-by: Muvaffak Onus <onus.muvaffak@gmail.com>
* Remove Establisher mechanism and port existing
establisher to Initializer.
Signed-off-by: Muvaffak Onus <onus.muvaffak@gmail.com>
* Fix claim's external name annotation propagation to
the managed resource.
Signed-off-by: Muvaffak Onus <onus.muvaffak@gmail.com>
This allows us to determine whether an external resource needs updating inside
the Observe method, which must already get a fresh copy of the external resource
from the cloud provider API in order to determine whether it exists and update
the managed resource's status.
Signed-off-by: Nic Cope <negz@rk0n.org>
Nic Cope