-
Notifications
You must be signed in to change notification settings - Fork 218
Support for optional @Dependent resources #2238
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
Comments
Hi @Javatar81 , not sure if I'm following every aspect of this, but are you suggesting that the controller would update the ClusterServiceVersion related to itself? So would update the |
Hi @csviri no I am not suggesting this since the CSV is static and it would not make sense to update it at runtime. I just propose to omit the CRD from the |
I think this makes sense, yes and covers rather common use cases. |
Ok so was confused about OLM part, since it's not related to the core. But otherwise this makes completely sense. just a note: for standalone workflows this won't work / not sure if we want to actually support it there explicitly. |
👍🏼
Why not? Could you elaborate why? |
So there is now no layer where that is processed properly, where this would create the activation condition. But will take a look maybe could be added elegantly also there. |
Just a note: since checking if the CRD is present, ideally there is an informer for CRD-s registered. This could be done seamlessly in the background if there is at least one dependent with This would look like: @ControllerConfiguration(
workflow = @Workflow(
registerCRDInformerEventSource = false,
dependents = {
@Dependent(...),
@Dependent(...)}
) (by default would be true) |
If there is no Informer for CRD, would be good to have an option to configure the refresh interval. |
Regarding a the refresh we might want to provide some sensible default but a customizable solution, similar to retries. checking the CRD every 15s seconds for 10 minutes for example; then just check it in every hour or so. |
For now we just added an generic activation condition after a debate if the optional flag would lead to some confusion. We revisit this later in case, will close the issue for now. |
In the current version JOSDK assumes when registering a dependent resource that it's managed dependent resource type (e.g. a
ConfigMap
orServiceMonitor
) is available and hence required for the operator. In most cases this holds true but for certain resource types (e.g. theServiceMonitor
) the dependency could be less strict when they are not available. The usual scenario with a required dependent looks like this:ServiceMonitor
CRD is not available in the cluster@Dependent
managing theServiceMonitor
ServiceMonitor
CRD via theClusterServiceVersion.spec.customresourcedefinitions.required
fieldSo far so good, but how could I implement the following behavior:
@Dependent
managing theServiceMonitor
ServiceMonitor
CRD via theClusterServiceVersion.spec.customresourcedefinitions.required
fieldactivationCondition
that checks whetherServiceMonitor
API is available in the cluster6a. The operator does not fail but ignores that it is not able to reconcile the
ServiceMonitor
because it is just optional.6b. The user installs the ServiceMonitor manually because she wants to use it and because it is available now, the operator will reconcile the
ServiceMonitor
(activationCondition
holds true)I see this type of optional dependencies in many official OpenShift operators, e.g. the ServiceMesh operator that interacts with Kiali. Kiali is not in the
ClusterServiceVersion.spec.customresourcedefinitions.required
. It must be installed before the ServiceMesh operator is installed.A potential solution could be to introduce a new attribute optional:
@Dependent(type = ServiceMonitorDependentResource.class, optional = true)
This attribute would add an
activationCondition
behind the scenes that checks for the API of the CRD and it would lead to omission of the CRD in theClusterServiceVersion.spec.customresourcedefinitions.required
array.The text was updated successfully, but these errors were encountered: