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

Add details to design for the new field #117

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 33 additions & 15 deletions enhancements/sig-policy/89-operator-policy-kind/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ fields based on specified defaults in the operator's PackageManifest. For exampl
default channel to possibly be different on different clusters, reflecting the default channel in
each cluster's catalog. It is not allowed to set `spec.subscription.installPlanApproval`; the policy
will determine and set a value for that field based on the policy's `spec.versions` and
`spec.updgradeApproval` settings.
`spec.upgradeApproval` settings.

In "musthave" mode, the `spec.versions` list specifies what installed versions are considered
Compliant when the policy is in `inform` mode, and which InstallPlans can be approved when in
Expand All @@ -154,14 +154,13 @@ The `spec.upgradeApproval` field specifies whether an enforced "musthave" policy
upgrade InstallPlans for the operator. *It has no effect when the policy is in "mustnothave" mode*.
This only affects InstallPlans for operators that are already installed on the cluster,
which upgrade or replace the operator; initial InstallPlans for an operator can be approved
regardless of this flag. Allowed values here include `Automatic` and `None`. If not set, the
controller will behave as if it were set to `Automatic`.
regardless of this flag. Allowed values here include `Automatic` and `None`.

Only when the policy is operating as if `spec.upgradeApproval` was set to `Automatic`, and the
`spec.versions` field is empty (allowing all versions of the operator) will the subscription
managed by this policy have its `installPlanApproval` field set to `Automatic`. Otherwise, the field
will be set to `Manual`, but it should be noted that this controller will approve InstallPlans
matching the desired versions automatically.
Only when the policy's `spec.upgradeApproval` is set to `Automatic`, and the `spec.versions` field
is empty (allowing all versions of the operator) will the subscription managed by this policy have
its `installPlanApproval` field set to `Automatic`. Otherwise, the field will be set to `Manual`,
but it should be noted that this controller will approve InstallPlans matching the desired versions
automatically.

The `spec.removalBehavior` field allows configuration of what might be removed by the controller
when the policy is in "mustnothave" mode. *It has no effect when the policy is in "musthave" mode.*
Expand Down Expand Up @@ -344,10 +343,10 @@ spec:
channel: stable
name: strimzi-kafka-operator
namespace: openshift-operators
installPlanApproval: Manual
source: community-operators
sourceNamespace: openshift-marketplace
startingCSV: strimzi-cluster-operator.v0.35.0
upgradeApproval: None
versions:
- strimzi-cluster-operator.v0.35.0
```
Expand All @@ -373,10 +372,10 @@ spec:
channel: stable
name: strimzi-kafka-operator
namespace: openshift-operators
installPlanApproval: Manual
source: community-operators
sourceNamespace: openshift-marketplace
startingCSV: strimzi-cluster-operator.v0.35.0
upgradeApproval: Automatic
versions:
- strimzi-cluster-operator.v0.35.0
- strimzi-cluster-operator.v0.35.1
Expand All @@ -401,10 +400,10 @@ spec:
channel: stable
name: strimzi-kafka-operator
namespace: openshift-operators
installPlanApproval: Manual
source: community-operators
sourceNamespace: openshift-marketplace
startingCSV: strimzi-cluster-operator.v0.35.0
upgradeApproval: None
versions:
- strimzi-cluster-operator.v0.35.0
statusConfig:
Expand Down Expand Up @@ -454,9 +453,9 @@ spec:
channel: stable
name: strimzi-kafka-operator
namespace: openshift-operators
installPlanApproval: Automatic
source: community-operators
sourceNamespace: openshift-marketplace
upgradeApproval: Automatic
statusConfig:
catalogSourceUnhealthy: NonCompliant
deploymentsUnavailable: NonCompliant
Expand Down Expand Up @@ -492,9 +491,9 @@ spec:
channel: stable
name: strimzi-kafka-operator
namespace: strimzi-app-one
installPlanApproval: Automatic
source: community-operators
sourceNamespace: openshift-marketplace
upgradeApproval: Automatic
```

#### Story 6
Expand All @@ -515,9 +514,9 @@ spec:
channel: stable
name: strimzi-kafka-operator
namespace: strimzi-app-one
installPlanApproval: Automatic
source: community-operators
sourceNamespace: openshift-marketplace
upgradeApproval: Automatic
removalBehavior:
operatorGroups: DeleteIfUnused
subscriptions: Delete
Expand Down Expand Up @@ -547,9 +546,9 @@ spec:
channel: stable
name: strimzi-kafka-operator
namespace: strimzi-app-one
installPlanApproval: Automatic
source: community-operators
sourceNamespace: openshift-marketplace
upgradeApproval: Automatic
versions:
- "strimzi-cluster-operator.v0.35.0"
removalBehavior:
Expand Down Expand Up @@ -597,6 +596,25 @@ setting, then that InstallPlan will be approved. If an InstallPlan is created fo
installations, it will not be approved by this controller, and it might cause an OperatorPolicy to
become NonCompliant.

#### Addition of the upgradesApproval field

An initial implementation *required* `spec.subscription.installPlanApproval` to be set to either
`Automatic` or `Manual`, but that setting was believed to be confusing. When set to `Automatic`, the
contoller needed to override that setting in the Subscription in order to restrict the possible
upgrades, and approve any upgrades matching the `versions` specified in the policy. And when set to
`Manual`, the controller would still approve those upgrades without user input, ie "automatically".

This potential confusion is resolved by *forbidding* the user from setting the
`spec.subscription.installPlanApproval` field in the subscription, while providing a similar setting
via `spec.upgradeApproval`. The revised design provides more details on the interactions of this
setting and the other fields. This provides more control to the user, since they can then separate
the initial install approval from any future upgrades.

In general, adding a required field could be considered a breaking change, but here we will *not*
increment the API version. Instead, the controller will validate that the new field is set properly,
and that `spec.subscription.installPlanApproval` is not set. "Old" instances of an OperatorPolicy
which do not follow the new requirements will be NonCompliant, and explain the required change.

### Risks and Mitigation

### Open Questions [optional]
Expand Down
Loading