enhancements: compound-bundle-constraints

[estroz]

Signed-off-by: Eric Stroczynski ericstroczynski@gmail.com

[estroz]

/cc @joelanford @kevinrizza @dinhxuanvu

[estroz]

/cc @dmesser

[estroz]

Any showstoppers for this getting merged? The plan is to start this feature off in alpha, so if changes to the spec are needed based on the higher level constraint vs. property discussion, they can be made easily and I'll update this EP then.

[joelanford]

Any showstoppers for this getting merged? The plan is to start this feature off in alpha, so if changes to the spec are needed based on the higher level constraint vs. property discussion, they can be made easily and I'll update this EP then.

Can you call this out specifically in the EP as a risk or open question, and then describe what the remediation will be?

It seems like this is well-defined enough to begin implementation, but if we're pretty confident that the syntax will change (even slightly), I think we'd do ourselves a favor if we kept the code related to parsing the constraint in a separate branch so it doesn't get merged downstream until we're confident it aligns with the discussion in #91.

Do you think its worthwhile running through another prototype demo based on what exists here to get something concrete in front of folks. I'd be interested in getting this on the agenda of one of our upstream meetings to get community feedback.

@cdjohnson Any comments/questions?

[estroz]

Do you think its worthwhile running through another prototype demo based on what exists here to get something concrete in front of folks. I'd be interested in getting this on the agenda of one of our upstream meetings to get community feedback.

Most definitely. Let me update the POC and add that to the agenda for the next OLM upstream meeting.

[perdasilva]

One thing that's kind of bother me a little is the interchange b/w property and constraint. E.g. if I just want a simple package level constraint, then that's a property. The same structure however would be re-used under constraint in the nested example. Maybe we should just use property everywhere?

[cdjohnson]

Question on Interoperability:

• Older versions of OLM (compound constraints are not known) will ignore the new constraint properties and continue to honor the old required properties at the root level.
• Newer versions of OLM (compound constraint aware) will honor BOTH the new and old properties.

I think we need a way to signal OLM to ignore the old properties if the new properties are present, or use the fact that there is an olm.all|any|none.required property at the top level to IGNORE the non-constraint properties at the top. It seems like we would want to deprecate the ability specify constraints at the top level and always express them in a compound dependency.

For example, if I wanted to build a Bundle that works on both, I could do something like this:

- type: olm.package.required
  value:
    description:  This is for legacy versions of OLM.
    packageName: bar
    versionRange: '>=1.0.0'
- type: olm.gvk.required
  value:
    description:  This is for legacy versions of OLM.
    group: etcd.database.coreos.com
    kind: EtcdBackup
    version: v1beta2
- type: olm.all.required
  value:
    constraints:
      - type: olm.package.required
        value:
          packageName: bar
          versionRange: '>=1.0.0'
      - type: olm.none.required
        value:
          - type: olm.package.required
            value:
              description: This package provides the same GVK, but I don't work with this one.
              packageName: brokenbar
              versionRange: '>=1.0.0'
      - type: olm.gvk.required
        value:
          description:  This is for legacy versions of OLM.
          group: etcd.database.coreos.com
          kind: EtcdBackup
          version: v1beta2

If the new version of OLM honors BOTH the old and new formats, then the new compound constraints will be ignored in this case.

[cdjohnson]

Question on resolver scope: Is each constraint applied to a single bundle? Or on the entire visible domain of operators (catalog sources and installed bundles)?

Example: I want GVK and Package to be served by the same bundle. Not by two different bundles.

[estroz]

One thing that's kind of bother me a little is the interchange b/w property and constraint. E.g. if I just want a simple package level constraint, then that's a property. The same structure however would be re-used under constraint in the nested example. Maybe we should just use property everywhere?

@perdasilva this is being resolved in https://github.com/operator-framework/enhancements/pull/91#issuecomment-959926059. All constraints are properties, but not all properties are constraints, hence the distinction.

I think we need a way to signal OLM to ignore the old properties if the new properties are present, or use the fact that there is an olm.all|any|none.required property at the top level to IGNORE the non-constraint properties at the top.

@cdjohnson I think this is a good idea for backwards-compatibility of bundles with OLM. The resolver can infer what to use by the presence of a compound constraint. I will say though, mixing both "legacy" and compound constraints in this way is confusing. I'll call out that this behavior needs to be documented and have warnings logged when this scenario is encountered.

Is each constraint applied to a single bundle? Or on the entire visible domain of operators (catalog sources and installed bundles)?

If I understand what you're asking, the latter. This is the same scope as for existing constraints.

[cdjohnson]

@estroz

Is each constraint applied to a single bundle? Or on the entire visible domain of operators (catalog sources and installed bundles)?

If I understand what you're asking, the latter. This is the same scope as for existing constraints.

After re-reading and talking to @dmesser, it appears that each Constraint Root (top level property) must evaluate to true for a single bundle. This wasn't clear to me until I re-read the old resolver EP. Essentially each Tree of constraints is evaluated independently against each bundle. So, by having a Compound Constraint they must evaluate to TRUE for a single bundle. I was incorrectly (I hope) thinking that the compound constraints were applied more broadly. I think it would be helpful to re-describe how the resolver uses this information (one tree at a time).

[cdjohnson]

I think we need a way to signal OLM to ignore the old properties if the new properties are present, or use the fact that there is an olm.all|any|none.required property at the top level to IGNORE the non-constraint properties at the top.

@cdjohnson I think this is a good idea for backwards-compatibility of bundles with OLM. The resolver can infer what to use by the presence of a compound constraint. I will say though, mixing both "legacy" and compound constraints in this way is confusing. I'll call out that this behavior needs to be documented and have warnings logged when this scenario is encountered.

@estroz I can think of a few alternatives:

1. Force catalogs and bundles to introduce OLM-Version-Specific versions of itself. Not a fan of this if we can help it.
2. Add an additional property to the legacy top-level value to explicitly ignore it by the "new compound-enabled resolver", to make it a bit more explicit.

Example:

- type: olm.package.required
  value:
    description:  This is for legacy versions of OLM.
    packageName: bar
    versionRange: '>=1.0.0'
    ignore: true

Old OLM versions ignore the ignore: true

[estroz]

Essentially each Tree of constraints is evaluated independently against each bundle. So, by having a Compound Constraint they must evaluate to TRUE for a single bundle. I was incorrectly (I hope) thinking that the compound constraints were applied more broadly.

Not exactly. A single bundle may never satisfy all constraints because one or more may reference GVKs/a package outside of that bundle's, so the resolver chooses a set of bundles from the universe (cluster) that satisfy all of a bundle's constraints.

I can think of a few alternatives

Thanks, it's worth calling out alternatives. I am not a fan of either approach though. The first makes ignoring top-level constraints in new OLM versions superfluous, since you're breaking backwards-compat anyway. The second adds a field with no context: why is this field ignored, and when should it be ignored?

There's a change currently being worked on by @dinhxuanvu in #91 that moves all constraints under the olm.constraints property. The "legacy" top-level constraints will be respected unless that property is defined, as we're discussing here. This change combined with docs and logs should raise this behavior's visibility appropriately.

[joelanford]

What effect does this EP have on the current GRPC API, which has requiredAPIs and dependencies fields in the Bundle object?

[estroz]

What effect does this EP have on the current GRPC API, which has requiredAPIs and dependencies fields in the Bundle object?

Is this change not being handled by #91? I would expect that olm.constraint's value is just sent wholesale over the gRPC API as a dependency, and requiredAPIs is left as-is since there isn't a good way to say what is required anymore. How would requiredAPIs be populated by a cel expression, for example?

[joelanford]

Is this change not being handled by #91? I would expect that olm.constraint's value is just sent wholesale over the gRPC API as a dependency, and requiredAPIs is left as-is since there isn't a good way to say what is required anymore. How would requiredAPIs be populated by a cel expression, for example?

I'm not sure #91 specifically covered this topic. Maybe we should revisit it there?

In general, this all makes sense. I'm just worried about something on the consuming end that doesn't know that requiredGVKs and dependencies are now a little more complicated and may not fully describe the constraints like they used to. I wonder if these fields need to be deprecated from the GRPC API.

[estroz]

Since #91 introduced olm.constraint and cel we should revisit it there.

I wonder if these fields need to be deprecated from the GRPC API.

Agreed, and perhaps a constraints field should be added to the API with contains all requiredAPIs plus the serialized values of new constraints. This further enforces the idea that constraints are properties and arbitrary, but can now be explicitly differentiated from properties as olm.constraints.

[estroz]

Updates:

• Allow gvk and package to be directly specified as olm.constraint values
• OLM will interpret old and new constraints simultaneously
• List OLM being able to choose between constraint schemas as an alternative

[joelanford]

/approve

[joelanford]

/approve cancel

One last question is around Tech Preview vs. GA. I'm personally satisfied with the state of this EP being GA'd immediately. If others feel we need a release or two of Tech Preview time, I think we may need an update to this EP clarifying a few things about the Tech Preview state of this feature:

• Is OLM upgradable if the feature is enabled?
• How can a customer enable the feature gate? (OLMConfig object? CVO Unmanaged + manual change to OLM deployment?)
• What would happen if a bundle specifies compound constraints but the feature is not enabled?

[joelanford]

/approve

Great work @estroz