✨ Workload conditions

[bhperry]

Summary

Adding the necessary type changes to ManifestWork to support the workload conditions and completion feature.

Related issue(s)

open-cluster-management-io/enhancements#138

[qiujian16]

you might want to add a marker listMapKey: condition and set strategy to merge.

[qiujian16]

I am wondering whether with type WellKnownCompletions, we do not need to set any other field in this struct.
If so, we might have a nested struct and put all field not needed for WellKnownCompletions to it, e.g.

type ConditionRule struct {
    Condition string `json:"condition"`
    Type ConditionRuleType `json:"type"`
   // this could be a pointer, so when type is WellKnownCompletions, this is nil
    CEL *CELRule `json:"cel"`
}

type CELRule struct {
    Expressions []string `json:"expressions"`
    Message string `json:"message"`
    ....
}

[bhperry]

My original idea was that the standard message and condition for WellKnownCompletions could be overridden by the user, such that they could use the well known rules but set them to a custom condition/message.

But I do see the benefit to simplifying and using nesting for the CELRule. I am open to switching to this approach if that is what you prefer.

[qiujian16]

I prefer a simplified way, and if there is use case later that we need message override for WellKnownCompletions, we could add another field solely for WellKnownCompletions type to set them.

[qiujian16]

And it might be more consistent if the type is WellKnownCondition? so for completed condition, user should set

condition: Completed
type: WellKownCondition

It would be easier for us to extend later for other condition.

[bhperry]

And it might be more consistent if the type is WellKnownCondition? so for completed condition, user should set

condition: Completed
type: WellKownCondition

It would be easier for us to extend later for other condition.

This is interesting. My thinking with WellKnownCompletions was that since Completed is a specialized case, users would want to be able to set that separately from other future well known conditions.

Are you suggesting that a rule like this would select just a single rule that matches the given condition

- condition: Completed
  type: WellKnownCondition

and then a rule like this would select all well knowns?

- type: WellKnownCondition

[bhperry]

Options:

1. Reject in webhook validation
2. The last occurrence of a condition overrides all previous
3. The full rule for the condition becomes the logical AND of all expressions across the rules

[qiujian16]

might work if add

// +listType=map
// +listMapKey=type
// +listMapKey=condition

such that there is no duplicate setting. but need take a try,

[qiujian16]

@bhperry would you like to add // +listMapKey=type
and we are good to merge this.

[bhperry]

I don't think type should be a map key. Condition is required now, so using just that as map key ensures each condition only has one rule. If we add type then OCM has to handle the case where a condition has a WellKnown and custom CEL rule.

[qiujian16]

got it, it is fine if condition is required.

[qiujian16]

/approve
only is to check if #362 (comment) works

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bhperry, qiujian16

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [qiujian16]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[qiujian16]

/lgtm