operator-framework / enhancements

Apache License 2.0
9 stars 40 forks source link

Update compound-bundle-constraints enhancement #108

Closed tylerslaton closed 1 year ago

tylerslaton commented 2 years ago

Signed-off-by: Tyler Slaton tyslaton@redhat.com

Summary

This is an update to the compound-bundle-constraints EP that is intended to address a couple of things.

  1. Make it clearer what negation does
  2. Update the negation keyword from none to not

Note The change to update none to `not will be addressed in the code directly via other PR's.

This is being done to address the issue outlined in BZ 2034319.

benluddy commented 2 years ago

Does renaming none address the root of the confusion around this behavior? This feature as a whole provides a mechanism for specifying how to select candidates for dependency (i.e. solver.Dependency) constraints only. Dependencies -- as opposed to conflicts -- can only ever block the installation of the bundle that declares the dependency (this seems to be what Evan was referring to in https://github.com/operator-framework/enhancements/pull/97#discussion_r743127921).

I think this is better addressed by clarifying the high-level semantics of this property versus focusing specifically on negation.

cdjohnson commented 2 years ago

I'm still confused on how bundle authors can take advantage of compound bundle constraints AND the the legacy non-compound bundle constraints simultaneously. This is addressed in the Alternatives section, but not clear if this is required of the spec or not. I think we can just add the new, narrower constraint.

Example: Today, we oftentimes try to use both olm.gvk.required and olm.package.required, which works sufficiently in most cases, unless there are two packages that provide the same GVK. We've run into occasions where two separate third-parties try to OLM-Enable two operators and provide them in separate catalogs, meaning we want to try and constrain our operator to one of the packages that serves a GVK.

Today:

---
schema: olm.bundle
name: foo.v1.0.0
package: foo
properties:
- type: olm.package.required
  value:
    name: etcd
    versionRange: '>=1.0.0'
- type: olm.gvk.required
  value:
    group: etcd.database.coreos.com
    kind: EtcdBackup
    version: v1beta2

Now, we want to leverage the more narrow rule, which we would now provide both formats. Because the new compound constraint is ALSO evaluated, the narrower rule is honored:

schema: olm.bundle
name: foo.v1.0.0
package: foo
properties:
# For all versions of OLM (a package and GVK don't need to be in the same bundle):
- type: olm.package.required
  value:
    name: etcd
    versionRange: '>=1.0.0'
- type: olm.gvk.required
  value:
    group: etcd.database.coreos.com
    kind: EtcdBackup
    version: v1beta2
# Only recognized by new OLM, which limits solutions to a single bundle that provides both the package and gvk.
- type: olm.constraint
  value:
    failureMessage: All are required for Baz because...
    all:
      constraints:
      - failureMessage: Package bar is needed for...
        package:
          name: etcd
          versionRange: '>=1.0.0'
      - failureMessage: GVK EtcdBackup/v1beta2 is needed for...
        gvk:
          group: etcd.database.coreos.com
          version: v1beta2
          kind: EtcdBackup

It might be helpful to call out a best practice for creating semi-interoperable dependencies like this.

cdjohnson commented 2 years ago

Can you clarify if a bundle provider can specify these constraints in the dependencies.yaml or if it only works in the properties.yaml within the bundle image?

dinhxuanvu commented 2 years ago

@cdjohnson As you mentioned, for the most use cases, the current existing olm.gvk.required and olm.package.required will be sufficient. However, as a known behavior, you list a required gvk and a required package, it may get resolved into 2 separate dependent operators depending the content of the catalogsources (shared owned API, same packages in multiple catalogsources and etc). The all type will solve this issue by ensuring all of required dependencies underneath all will only get resolved into a single dependent operator if satisfied. So to me that's the main use case for this compound constraint. This is a new type of dependencies and it is not required or anything like that.

dinhxuanvu commented 2 years ago

Can you clarify if a bundle provider can specify these constraints in the dependencies.yaml or if it only works in the properties.yaml within the bundle image?

Currently, it should be declared in dependencies.yaml just like other existing dependency types. We want to keep the convention the same even though you can technically add the new type in properties.yaml and OLM will understand it.

cdjohnson commented 2 years ago

@dinhxuanvu:

...However, as a known behavior, you list a required gvk and a required package, it may get resolved into 2 separate dependent operators depending the content of the catalogsources (shared owned API, same packages in multiple catalogsources and etc). The all type will solve this issue by ensuring all of required dependencies underneath all will only get resolved into a single dependent operator if satisfied. So to me that's the main use case for this compound constraint. This is a new type of dependencies and it is not required or anything like that.

Yes, I understand that. I'm asking if we want to leverage the new constraint AND preserve the old behavior for old OLM resolvers: Is my example is valid? I'd like to tell ALL of our products to follow this example to allow a more exact constraint when possible, and the lesser constraint when not possible.

cdjohnson commented 2 years ago

@dinhxuanvu

Currently, it should be declared in dependencies.yaml just like other existing dependency types. We want to keep the convention the same even though you can technically add the new type in properties.yaml and OLM will understand it.

Can we add this to the specification, so it's obvious to all readers? It seems like examples showing dependencies.yaml is more relevant to most, rather than the File Based Catalog examples.

dinhxuanvu commented 2 years ago

@dinhxuanvu:

...However, as a known behavior, you list a required gvk and a required package, it may get resolved into 2 separate dependent operators depending the content of the catalogsources (shared owned API, same packages in multiple catalogsources and etc). The all type will solve this issue by ensuring all of required dependencies underneath all will only get resolved into a single dependent operator if satisfied. So to me that's the main use case for this compound constraint. This is a new type of dependencies and it is not required or anything like that.

Yes, I understand that. I'm asking if we want to leverage the new constraint AND preserve the old behavior for old OLM resolvers: Is my example is valid? I'd like to tell ALL of our products to follow this example to allow a more exact constraint when possible, and the lesser constraint when not possible.

Your example looks valid to me. However, on the second one, it seems you only need the olm.constraint dependency and you can remove the olm.gvk.required and olm.package.required dependencies as they are duplicates. From what I gather, you want a single dependent operator solution for the two requirements so the all type alone is sufficient.

cdjohnson commented 2 years ago

@dinhxuanvu:

Your example looks valid to me. However, on the second one, it seems you only need the olm.constraint dependency and you can remove the olm.gvk.required and olm.package.required dependencies as they are duplicates. From what I gather, you want a single dependent operator solution for the two requirements so the all type alone is sufficient.

My point, is that I want this same bundle to be able to be installed and the dependencies understood on OpenShift 4.6 through 4.9, which won't have OLM that understands olm.constraint. So, I need to supply BOTH the old and new format simultaneously OR i need to create different bundles with different dependency.yaml files for different versions of OpenShift (OLM), which means I need different catalogs, etc.

benluddy commented 2 years ago

@cdjohnson It's fine to specify both. There are global invariants on GVK providers, so only one operator can provide any given GVK. If your operator requires A, B, and (A ^ B), it's effectively equivalent to requiring only (A ^ B) for a version of catalog-operator that understands (A ^ B).

cdjohnson commented 2 years ago

There are global invariants on GVK providers, so only one operator can provide any given GVK

This is likely true in how the resolver produces a solution. However, in practice this is not true in the context of the universe (my understanding of "global" which we cannot control) of public catalogs, where anyone can build and publish an OLM-Enabled operator. For example, when submitting an Operator foo to the Red Hat certification program, that program appends -certified to the name of the package. So if that operator was NOT certified AND certified, there are two packages that serve the same GVK. THere are also examples of 3rd parties delivering their own version of an OLM-Enabled operator for the same GVK's.... This is the reason we're trying to provide this constraint, and why I really like this feature.

benluddy commented 2 years ago

Yes, exactly right. I was specifically referring to namespace resolution here, because that is what prevents the selection of a solution like { A1 B1 A2 }. If that resolution invariant didn't exist, then it would not be safe to keep the existing dependencies alongside a compound one.