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

Optional OLM supported resources #2375

Open
fgiloux opened this issue Sep 24, 2021 · 6 comments
Open

Optional OLM supported resources #2375

fgiloux opened this issue Sep 24, 2021 · 6 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. triaged Issue has been considered by a member of the OLM community

Comments

@fgiloux
Copy link
Contributor

fgiloux commented Sep 24, 2021

Feature Request

Is your feature request related to a problem? Please describe.
Today when the API for the resource in the manifest directory does not exist in the cluster where the operator gets installed the InstallPlan fails.
ServiceMonitor and VerticalPodAutoscaler are among the supported resources. These resources allow the operator authors to provide more advanced features (observability, autoscaling). These features are however not part of the core functionalities of most operators. As such the operator author may want to leverage them but not enforce them so that the operator bundle can be installed on clusters where they are not available.

Describe the solution you'd like
This feature request proposes to have a knob for the operator author to mark some of the resources in the bundle manifest directory as optional. If the API is not available in the target cluster at installation time:

  • The information will get surfaced to the cluster admin through logs and any other suitable way
  • The InstallPlan will go on with the installation
@fgiloux fgiloux added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 24, 2021
@cdjohnson
Copy link

@fgiloux I'm actually working on a couple of enhancement proposals right now around this. I gave a preview last week.

I'd be interested in your feedback when I submit the PR. There will be two EP's:

  1. Define how to declare non-required (optional) dependencies and how consumers (OLM, OPM, and others) should interpret this data.
  2. Describe how to Provision optional dependencies if they are not provisioned automatically by OLM.

In the link I provided above, you can see the preview of the first EP.

@fgiloux
Copy link
Contributor Author

fgiloux commented Sep 29, 2021

@cdjohnson I had a look at: https://github.com/cdjohnson/opf-enhancements/blob/master/enhancements/olm-optional-dependency-declaration.md
I agree with you that my feature request is about optional dependencies at the end of the day but it is way less ambitious than your EP. Your EP is about explicit dependencies. This feature request is about implicit dependencies: the ones you create when you put a resource in the manifest directory. Therefore you don't have to specify it in dependencies.yaml or the CRD.
For your EP to work you may however need this feature. The knob I am mentioning in this feature request may well be the attribute you specify in your EP, meaning that OLM moves away of implicit dependencies.
If a dependency is optional the operator author may still provide manifests of the optional resource in the manifest directory. If the cluster the bundle gets deployed to has the CRD for the resource all is fine, if it can get installed beforehand that's good but if the dependency cannot be satisfied the InstallPlan should not fail. It would today when it tries to create the optional resource from the manifest directory.

Here is some feedback on your EP:

  • I like proposal 1 better: dependencies are dependencies optional or not, I don't see reasons for having different types
  • I would first start with "required" (current behavior) and "optional", which I interpret as: ignore the resource if the type is not available. Maybe you have something concrete in mind but I am not sure there is a need for something in between. It is always easier to add new options when needed rather than to remove unnecessary ones later on.
    Also I am not sure of what you mean with "Clients can override the default behavior."

@cdjohnson
Copy link

@fgiloux Thanks for the feedback. I understand your use case as follows (based on the current state of the EP):

Build installplan from bundle:

  1. For each manifest in the bundle:
    1. If the manifest resource's GVK API is Present, apply it.
    2. If the manifest resource's GVK API is not present:
      1. Does the resource's GVK API specify an intent?
        1. If absent or required, fail to resolve
        2. If optional do not apply the resource.
        3. If recommended attempt to provision the Operator that serves the GVK API and apply the resource.

Also I am not sure of what you mean with "Clients can override the default behavior."
There are two aspects: Declaring the dependencies and the intent somehow and then how clients can honor those dependency intents. I'm saying: That a client may elect to override the intended behavior (e.g. make a dependency required even though its intent is optional). Several package manager CLI's have this capability.

@cdjohnson
Copy link

Related issue:
#1860

@dinhxuanvu dinhxuanvu added the triaged Issue has been considered by a member of the OLM community label Sep 30, 2021
@fgiloux
Copy link
Contributor Author

fgiloux commented Oct 7, 2021

@cdjohnson and all I have created a pull request for an enhancement proposal that addresses this feature request. I believe it would play well with the work being done on optional dependencies.
Details here: operator-framework/enhancements#95

@timflannagan
Copy link
Contributor

Just a quick note: the enhancement Frederic linked had merged yesterday.

@timflannagan timflannagan linked a pull request Feb 11, 2022 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. triaged Issue has been considered by a member of the OLM community
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants