✨ 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]

need to set an kubebuilder:validation:Enum marker

[bhperry]

This is already set on the typedef https://github.com/open-cluster-management-io/api/pull/362/files#diff-22c62942813270e1c12182a840cacc1358496a602668f058ef6aff4d0febbceaR156-R157

There is a mix of styles for enums. Some set on type, some on usage. I based this off of type FeedBackType string so followed that style.

[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]

How do you feel about the idea I mentioned previously? Seems like it is a nice middle ground of making things easy by default but also allowing individual customization

- condition: Completed
  type: WellKnownCondition  # Single well known condition

- type: WellKnownCondition  # All well known conditions

[qiujian16]

I think making condition as a required field and mergeKey will make setting type only impossible. And I think we have to make condition as the mergekey for better merge strategy and ensure user does not input duplicated conditions

[qiujian16]

question is what happens when user set

- type: WellKnownCondition 
- type: CEL
   condition: Completed
   expressions: ....

or

- type: CEL
   condition: Completed
    expressions: ... 
- type: CEL
   condition: Completed
   expressions: ....

[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]

/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