Skip to content

Support for optional @Dependent resources #2238

Closed

Description

@Javatar81

In the current version JOSDK assumes when registering a dependent resource that it's managed dependent resource type (e.g. a ConfigMap or ServiceMonitor) is available and hence required for the operator. In most cases this holds true but for certain resource types (e.g. the ServiceMonitor) the dependency could be less strict when they are not available. The usual scenario with a required dependent looks like this:

  1. The ServiceMonitor CRD is not available in the cluster
  2. My operator defined a @Dependent managing the ServiceMonitor
  3. The operator expresses its requirement for the ServiceMonitor CRD via the ClusterServiceVersion.spec.customresourcedefinitions.required field
  4. If I install the operator with this requirement, the Prometheus operator will be installed, too.

So far so good, but how could I implement the following behavior:

  1. The ServiceMonitor CRD is not be available in the cluster
  2. My operator defined a @Dependent managing the ServiceMonitor
  3. The operator does NOT define a requirement for the ServiceMonitor CRD via the ClusterServiceVersion.spec.customresourcedefinitions.required field
  4. Without the requirement, the Prometheus operator will NOT be installed.
  5. The operator defines an activationCondition that checks whether ServiceMonitor API is available in the cluster
    6a. 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 the ClusterServiceVersion.spec.customresourcedefinitions.required array.

Activity

  1. csviri commented on Feb 7, 2024

    @csviri
    Collaborator

    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 ClusterServiceVersion.spec.customresourcedefinitions.required ?

  2. Javatar81 commented on Feb 7, 2024

    @Javatar81
    Author

    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 ClusterServiceVersion.spec.customresourcedefinitions.required array if it is defined as an optional dependency.

  3. metacosm commented on Feb 12, 2024

    @metacosm
    Collaborator

    I think this makes sense, yes and covers rather common use cases.

  4. csviri commented on Feb 12, 2024

    @csviri
    Collaborator

    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.

  5. added this to the 5.0 milestone on Feb 12, 2024
  6. metacosm commented on Feb 12, 2024

    @metacosm
    Collaborator

    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?

  7. csviri commented on Feb 12, 2024

    @csviri
    Collaborator

    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.

  8. self-assigned this
    on Feb 13, 2024
  9. csviri commented on Feb 13, 2024

    @csviri
    Collaborator

    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 optional=true. But also could be just checked directly if the informer is not desired (like no right to watch? ). For this we would need a feature flag similar to this:
    #1898

    This would look like:

    @ControllerConfiguration(
        workflow = @Workflow( 
              registerCRDInformerEventSource = false,
              dependents = {
                  @Dependent(...),
                  @Dependent(...)}
        )

    (by default would be true)

  10. csviri commented on Feb 13, 2024

    @csviri
    Collaborator

    If there is no Informer for CRD, would be good to have an option to configure the refresh interval.

  11. linked a pull request that will close this issuefeat: optional dependent resource #2240on Feb 13, 2024
  12. csviri commented on Feb 13, 2024

    @csviri
    Collaborator

    Regarding a the refresh we might want to provide some sensible default but a customizable solution, similar to retries.
    To be honest the usefulness dynamic part of the activation condition will be (in my experience) limited to for the initialization phase, so the CRDs that a platform usually support does not change that frequently, so would be reasonable. But the ordering of CRD apply and operator installation might matter, so what by default make sense to me is like:

    checking the CRD every 15s seconds for 10 minutes for example; then just check it in every hour or so.

  13. csviri commented on Jun 19, 2024

    @csviri
    Collaborator

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions