Change Naming of `reconcilePrecondition` for v5
It can be confusing that in the precondition there is reconcile included, this not really describes the deletion that happens if this condition does not hold.
Maybe it should be just condition ?
This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.
This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.
This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.
When the reconcileCondition holds true first and then later changes to false, the secondary resource is deleted. This is clearly described in the Javadocs. Two questions regarding this:
- Why is it deleted at all? The name would indicate that it is just not reconciled.
- What happens when the reconcileCondition holds false and then changes to true? Is the secondary registered again? This is not stated in the docs.
Maybe instead of renaming the method you could change it is logic. I don't know why it was implemented like that, is there a specific use case this was needed for or is it just an optimization?
@Javatar81 the idea is that the condition is a barrier for the secondary reconciliation meaning that if the condition is not true then the secondary shouldn't exist and therefore not reconciled (because, if it existed, it certainly should be reconciled to decide whether or not its state is conform to the desired state described by the primary resource). I do agree, though, that this is somewhat confusing.
Let's assume this scenario:
https://github.com/operator-framework/java-operator-sdk/blob/af7706452752dadcecaccf549640066c795311d3/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java#L22-L22
We want in the CR a flag to create an Ingress or not. If we want to describe such cases, we need a conditional resource, since some might want to change this flag from true to false. Thus remove the Ingress.
Side note: not reconciling a resource does not make sense, only case when it happens or might happen if a resource B depends on resource A, where A has a ready post condition; and before it already happened that A was ready and B reconciled, but now A is not ready (like new version deployment happening or such), and B is not reconciled until the A is ready. Otherwise workflow reconciles in a run all the resources.
So in this regard the reconcile word in precondition I think is a bit misleading since the condition does not prevent the reconciliation, but basically we are describing if the resource should exist or not.
In #2063 we discussed the activationCondition. I needed this because I had the additional requirement that the API for the dependent resource might not be available in the cluster and even with reconcilePrecondition the operator calls the API to access the resource type.
But besides that, what are the differences between activationCondition and reconcilePrecondition? I could replace an reconcilePrecondition by an activationCondition but not vice versa, right?
The activation condition is very limited atm, for example you cannot make resources properly of same type with activation condition. (Will describe more the limitations in the docs). If you know that the resources will be always present on the cluster you can replace the activationCondition with reconcilePreCondition. Vice versa is not working always, just in some limited use cases.
Technically the differences is how the event source is registered, when a resource has an activation condition, the event source is not registered while the condition is not true. This is much more trickier than it might sound.
If the limitations would be solved the activation condition functionally replace in theory the reconcile condition, but registering dynamically the event sources is less efficient (although the differences might not be noticable).
We will explore more the possibilities of the activation condition. TBH it would be much easier to support it with just the standalone dependent resources.
see discussion in the PR, we will stick with the original naming for now.