From 5b82635ef101e7c10b5fcbbcfb81315bb7a0da20 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Mon, 31 Jan 2022 16:39:03 -0800 Subject: [PATCH] config/v1/types_cluster_version: Add capabilties properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Roughly as described in [enhancement]. After discussion with Ben and David, we've made the following changes: # spec ## capabilities The [enhancement] didn't have an opinion on whether or not this should be a pointer. I've gone with pointer, because I'm fine allowing folks to leave this unset. The docs for this pointer property point out that there's no distinction between nil (Go, or for JSON, null) and an empty object (&ClusterVersionCapabilitiesSpec{} in Go, {} in JSON), so we don't have to rehash all the ClusterVersionCapabilitiesSpec children defaults here, where they'd likely go stale as folks update defaults within ClusterVersionCapabilitiesSpec or add new child properties. ### baselineCapabilitySet David prefered this name to the [enhancement]'s inclusionDefault, and Ben and I are fine with that name. David also prefered None to Exclude and vCurrent to Include, with additional values like v4.11 for "give me the 4.11 stuff, but not new 4.12 stuff, until I have time to look that over after I update to 4.12". That seems overly complicated to me, and also like we coulld add v4.11 later if folks felt None and vCurrent weren't convenient enough, but David wanted v4.11 out of the gate. We can always see how it plays out in production, and we can stop adding new v4.y forms if they aren't popular enough to be worth maintaining. There's an enum type to make it easy to validate, and hard to typo, these values. There's also a map, so consumers like the cluster-version operator who vendor the API repository can get the API-maintainer-intended capability membership for each set, now that those semantics are more complicated than all or nothing. There were a few ways we could have taken the property default here: a. Explicit +kubebuilder:default:=... . No option for folks to sit on the fence, or to adjust existing clusters later without the admin's explicit buy-in. But no ambiguity about what happens if the user has no opinion. b. omitempty, and docs for "we'll pick a sane default if you don't care", that don't commit to a specific default. Great for when we decide to change our default preference, because we don't need to hunt down buy-in from admins that have deferred to us. Not great for folks who are mildly curious about our current choice, but who still trust us to evolve the default (I think this set is nearly empty). c. omitempty, and docs for "the current default is A, but who knows tomorrow". Effectively (b), but also gives folks some information that's not actionable which can go stale as soon as they close their eyes. (a) makes sense if we are confident we will never want to change our default, and that seems plausible in this case. (b) makes sense when we think we might change our default, and I'm fine with that in this case too. I don't really understand the use case for (c), but as David points out, even if we do change the default, we don't expect to change it often, so maybe my personal take is off and there are a bunch of folks who are mildly curious about our current choice, but who still trust us to evolve the default. Anyhow, David's the approver, so we're going with (c). ### additionalEnabledCapabilities David prefered this name to the [enhancement]'s inclusionDefault, and Ben and I are fine with that name. In the [enhancement], Ben had intended to distribute the ability to create new capabilities to all manifest-providing repositories, simply by declaring the capability.openshift.io/name annotation. David was worried about validation, and also possibly about insufficiently scoped names between separate teams, so this pull request declares a central enum where API maintainers can review and approve new capability names, and work them into the appropriate entries in the set maps. The installer and cluster-version operator will have to bump their vendored API version after each addition to pick up the new changes, but new capability additions shouldn't be too frequent to make that particularly painful. ### exclude The [enhancement] also provided a way to drop specific capabilities from the selected set (inclusionDefault or baselineCapabilitySet). Ben still feels like that will be popular, but David is skeptical, and because we can always add a property in this space later without breaking backwards compatibility, we're leaving it off for now. # status ## capabilities The [enhancement] didn't have an opinion on whether or not this should be a pointer. I've gone with non-pointer, because we will always declare at least some capabilities (e.g. knownCapabilities), so users will be able to discover additional capabilities which they might want to enable in their cluster. ### enabledCapabilities David prefered this name to the [enhancement]'s include, and Ben's ok with that name. I'm not wild about 'Capabilities' in: status: capabilities: enabledCapabilities: ... but David was committed, citing the example of: FeatureGateSelection.FeatureSet Although FeatureGateSelection is consumed with less context: type FeatureGateSpec struct { FeatureGateSelection `json:",inline"` } I'd pushed back against the stuttering in [stutterPrecedent], but without success, and :shrug:, doesn't matter all that much if folks have to type a few redundant characters to use this property. ### knownCapabilities The [enhancement] had floated 'exclude'. There are a few capability sets in play for the status listings: * A is the set of all caps. * I is the set of included caps. * E is the set of excluded caps. * Each cap must be either included or excluded, so I and E are disjoint, and the union of I and E is A. So you can take any two of those three sets and construct the one you're missing: * A = I ∪ E * E = A - I * I = A - E If we have to pick two to include in status, picking I and E gives us all the data we need, and saves a few bytes by excluding the largest, which is A. But David preferred picking I (as enabledCapabilities) and A (as knownCapabilities) [knownCapabilities], so that's what we're doing in this commit. ### inclusionDefault The [enhancement] also provided a way to echo the spec set in an inclusionDefault status property. I've left that out of the status structure, because I'm using explicit, exhaustive list for enabledCapabilities and knownCapabilities there. The exhaustive lists will provide a convenient set (via A - I set subtraction) of "things you don't have right now, but which you could choose to install right now", so admins don't have to guess about their options there. With the exhaustive lists, reflecting the default setting didn't seem to add much useful information. And we can always add that property to the status structure later if we do decide it would be useful. ## conditions I have not created a constant with the status.conditions[] type that will be used to declare "we are installing a capability you have not asked for, because we don't support uninstalling capabilities, and that one was tainted in via your cluster's history". We can come back and declare that constant later if we want, although that's somewhat complicated by the fact that we use ClusterOperatorStatusCondition, and the new condition type would not be something that makes sense for ClusterOperator. [enhancement]: https://github.com/openshift/enhancements/blob/88cb7438f3ac0a8121e3cef87cb144097ece8cda/enhancements/installer/component-selection.md [knownCapabilities]: https://github.com/openshift/api/pull/1106#discussion_r799819632 [validation]: https://book.kubebuilder.io/reference/generating-crd.html#validation [statusPropertyNames]: https://github.com/openshift/api/pull/1106#discussion_r799819632 [stutterPrecedent]: https://github.com/openshift/api/pull/1106#discussion_r799879689 --- ...ersion-operator_01_clusterversion.crd.yaml | 42 +++++++++ config/v1/types_cluster_version.go | 89 +++++++++++++++++++ config/v1/zz_generated.deepcopy.go | 53 +++++++++++ .../v1/zz_generated.swagger_doc_generated.go | 22 +++++ 4 files changed, 206 insertions(+) diff --git a/config/v1/0000_00_cluster-version-operator_01_clusterversion.crd.yaml b/config/v1/0000_00_cluster-version-operator_01_clusterversion.crd.yaml index 477435fd617..66306bee055 100644 --- a/config/v1/0000_00_cluster-version-operator_01_clusterversion.crd.yaml +++ b/config/v1/0000_00_cluster-version-operator_01_clusterversion.crd.yaml @@ -52,6 +52,26 @@ spec: required: - clusterID properties: + capabilities: + description: capabilities configures the installation of optional, core cluster components. A null value here is identical to an empty object; see the child properties for default semantics. + type: object + properties: + additionalEnabledCapabilities: + description: additionalEnabledCapabilities extends the set of managed capabilities beyond the baseline defined in baselineCapabilitySet. The default is an empty set. + type: array + items: + description: ClusterVersionCapability enumerates optional, core cluster components. + type: string + enum: + - openshift-samples + x-kubernetes-list-type: atomic + baselineCapabilitySet: + description: baselineCapabilitySet selects an initial set of optional capabilities to enable, which can be extended via additionalEnabledCapabilities. If unset, the cluster will choose a default, and the default may change over time. The current default is vCurrent. + type: string + enum: + - None + - v4.11 + - vCurrent channel: description: channel is an identifier for explicitly requesting that a non-default set of updates be applied to this cluster. The default channel will be contain stable updates that are appropriate for production clusters. type: string @@ -133,6 +153,28 @@ spec: description: version is a semantic versioning identifying the update version. When this field is part of spec, version is optional if image is specified. type: string nullable: true + capabilities: + description: capabilities describes the state of optional, core cluster components. + type: object + properties: + enabledCapabilities: + description: enabledCapabilities lists all the capabilities that are currently managed. + type: array + items: + description: ClusterVersionCapability enumerates optional, core cluster components. + type: string + enum: + - openshift-samples + x-kubernetes-list-type: atomic + knownCapabilities: + description: knownCapabilities lists all the capabilities known to the current cluster. + type: array + items: + description: ClusterVersionCapability enumerates optional, core cluster components. + type: string + enum: + - openshift-samples + x-kubernetes-list-type: atomic conditionalUpdates: description: conditionalUpdates contains the list of updates that may be recommended for this cluster if it meets specific required conditions. Consumers interested in the set of updates that are actually recommended for this cluster should use availableUpdates. This list may be empty if no updates are recommended, if the update service is unavailable, or if an empty or invalid channel has been specified. type: array diff --git a/config/v1/types_cluster_version.go b/config/v1/types_cluster_version.go index 44e8677789c..2bd6c657015 100644 --- a/config/v1/types_cluster_version.go +++ b/config/v1/types_cluster_version.go @@ -68,6 +68,12 @@ type ClusterVersionSpec struct { // +optional Channel string `json:"channel,omitempty"` + // capabilities configures the installation of optional, core + // cluster components. A null value here is identical to an + // empty object; see the child properties for default semantics. + // +optional + Capabilities *ClusterVersionCapabilitiesSpec `json:"capabilities,omitempty"` + // overrides is list of overides for components that are managed by // cluster version operator. Marking a component unmanaged will prevent // the operator from creating or updating the object. @@ -113,6 +119,9 @@ type ClusterVersionStatus struct { // +required VersionHash string `json:"versionHash"` + // capabilities describes the state of optional, core cluster components. + Capabilities ClusterVersionCapabilitiesStatus `json:"capabilities"` + // conditions provides information about the cluster version. The condition // "Available" is set to true if the desiredUpdate has been reached. The // condition "Progressing" is set to true if an update is being applied. @@ -215,6 +224,86 @@ type UpdateHistory struct { // ClusterID is string RFC4122 uuid. type ClusterID string +// ClusterVersionCapability enumerates optional, core cluster components. +// +kubebuilder:validation:Enum=openshift-samples +type ClusterVersionCapability string + +const ( + // ClusterVersionCapabilityOpenShiftSamples manages the sample + // image streams and templates stored in the openshift + // namespace, and any registry credentials, stored as a secret, + // needed for the image streams to import the images they + // reference. + ClusterVersionCapabilityOpenShiftSamples ClusterVersionCapability = "openshift-samples" +) + +// ClusterVersionCapabilitySet defines sets of cluster version capabilities. +// +kubebuilder:validation:Enum=None;v4.11;vCurrent +type ClusterVersionCapabilitySet string + +const ( + // ClusterVersionCapabilitySetNone is an empty set enabling + // no optional capabilities. + ClusterVersionCapabilitySetNone ClusterVersionCapabilitySet = "None" + + // ClusterVersionCapabilitySet4_11 is the recommended set of + // optional capabilities to enable for the 4.11 version of + // OpenShift. This list will remain the same no matter which + // version of OpenShift is installed. + ClusterVersionCapabilitySet4_11 ClusterVersionCapabilitySet = "v4.11" + + // ClusterVersionCapabilitySetCurrent is the recommended set + // of optional capabilities to enable for the cluster's + // current version of OpenShift. + ClusterVersionCapabilitySetCurrent ClusterVersionCapabilitySet = "vCurrent" +) + +// ClusterVersionCapabilitySets defines sets of cluster version capabilities. +var ClusterVersionCapabilitySets = map[ClusterVersionCapabilitySet][]ClusterVersionCapability{ + ClusterVersionCapabilitySetNone: {}, + ClusterVersionCapabilitySet4_11: { + ClusterVersionCapabilityOpenShiftSamples, + }, + ClusterVersionCapabilitySetCurrent: { + ClusterVersionCapabilityOpenShiftSamples, + }, +} + +// ClusterVersionCapabilitiesSpec selects the managed set of +// optional, core cluster components. +// +k8s:deepcopy-gen=true +type ClusterVersionCapabilitiesSpec struct { + // baselineCapabilitySet selects an initial set of + // optional capabilities to enable, which can be extended via + // additionalEnabledCapabilities. If unset, the cluster will + // choose a default, and the default may change over time. + // The current default is vCurrent. + // +optional + BaselineCapabilitySet ClusterVersionCapabilitySet `json:"baselineCapabilitySet,omitempty"` + + // additionalEnabledCapabilities extends the set of managed + // capabilities beyond the baseline defined in + // baselineCapabilitySet. The default is an empty set. + // +listType=atomic + // +optional + AdditionalEnabledCapabilities []ClusterVersionCapability `json:"additionalEnabledCapabilities,omitempty"` +} + +// ClusterVersionCapabilitiesStatus describes the state of optional, +// core cluster components. +// +k8s:deepcopy-gen=true +type ClusterVersionCapabilitiesStatus struct { + // enabledCapabilities lists all the capabilities that are currently managed. + // +listType=atomic + // +optional + EnabledCapabilities []ClusterVersionCapability `json:"enabledCapabilities,omitempty"` + + // knownCapabilities lists all the capabilities known to the current cluster. + // +listType=atomic + // +optional + KnownCapabilities []ClusterVersionCapability `json:"knownCapabilities,omitempty"` +} + // ComponentOverride allows overriding cluster version operator's behavior // for a component. // +k8s:deepcopy-gen=true diff --git a/config/v1/zz_generated.deepcopy.go b/config/v1/zz_generated.deepcopy.go index 97ddfece0be..f28ca70d003 100644 --- a/config/v1/zz_generated.deepcopy.go +++ b/config/v1/zz_generated.deepcopy.go @@ -984,6 +984,53 @@ func (in *ClusterVersion) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterVersionCapabilitiesSpec) DeepCopyInto(out *ClusterVersionCapabilitiesSpec) { + *out = *in + if in.AdditionalEnabledCapabilities != nil { + in, out := &in.AdditionalEnabledCapabilities, &out.AdditionalEnabledCapabilities + *out = make([]ClusterVersionCapability, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterVersionCapabilitiesSpec. +func (in *ClusterVersionCapabilitiesSpec) DeepCopy() *ClusterVersionCapabilitiesSpec { + if in == nil { + return nil + } + out := new(ClusterVersionCapabilitiesSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterVersionCapabilitiesStatus) DeepCopyInto(out *ClusterVersionCapabilitiesStatus) { + *out = *in + if in.EnabledCapabilities != nil { + in, out := &in.EnabledCapabilities, &out.EnabledCapabilities + *out = make([]ClusterVersionCapability, len(*in)) + copy(*out, *in) + } + if in.KnownCapabilities != nil { + in, out := &in.KnownCapabilities, &out.KnownCapabilities + *out = make([]ClusterVersionCapability, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterVersionCapabilitiesStatus. +func (in *ClusterVersionCapabilitiesStatus) DeepCopy() *ClusterVersionCapabilitiesStatus { + if in == nil { + return nil + } + out := new(ClusterVersionCapabilitiesStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterVersionList) DeepCopyInto(out *ClusterVersionList) { *out = *in @@ -1025,6 +1072,11 @@ func (in *ClusterVersionSpec) DeepCopyInto(out *ClusterVersionSpec) { *out = new(Update) **out = **in } + if in.Capabilities != nil { + in, out := &in.Capabilities, &out.Capabilities + *out = new(ClusterVersionCapabilitiesSpec) + (*in).DeepCopyInto(*out) + } if in.Overrides != nil { in, out := &in.Overrides, &out.Overrides *out = make([]ComponentOverride, len(*in)) @@ -1054,6 +1106,7 @@ func (in *ClusterVersionStatus) DeepCopyInto(out *ClusterVersionStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + in.Capabilities.DeepCopyInto(&out.Capabilities) if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]ClusterOperatorStatusCondition, len(*in)) diff --git a/config/v1/zz_generated.swagger_doc_generated.go b/config/v1/zz_generated.swagger_doc_generated.go index 811becd56e2..020cdded5eb 100644 --- a/config/v1/zz_generated.swagger_doc_generated.go +++ b/config/v1/zz_generated.swagger_doc_generated.go @@ -548,6 +548,26 @@ func (ClusterVersion) SwaggerDoc() map[string]string { return map_ClusterVersion } +var map_ClusterVersionCapabilitiesSpec = map[string]string{ + "": "ClusterVersionCapabilitiesSpec selects the managed set of optional, core cluster components.", + "baselineCapabilitySet": "baselineCapabilitySet selects an initial set of optional capabilities to enable, which can be extended via additionalEnabledCapabilities. If unset, the cluster will choose a default, and the default may change over time. The current default is vCurrent.", + "additionalEnabledCapabilities": "additionalEnabledCapabilities extends the set of managed capabilities beyond the baseline defined in baselineCapabilitySet. The default is an empty set.", +} + +func (ClusterVersionCapabilitiesSpec) SwaggerDoc() map[string]string { + return map_ClusterVersionCapabilitiesSpec +} + +var map_ClusterVersionCapabilitiesStatus = map[string]string{ + "": "ClusterVersionCapabilitiesStatus describes the state of optional, core cluster components.", + "enabledCapabilities": "enabledCapabilities lists all the capabilities that are currently managed.", + "knownCapabilities": "knownCapabilities lists all the capabilities known to the current cluster.", +} + +func (ClusterVersionCapabilitiesStatus) SwaggerDoc() map[string]string { + return map_ClusterVersionCapabilitiesStatus +} + var map_ClusterVersionList = map[string]string{ "": "ClusterVersionList is a list of ClusterVersion resources.\n\nCompatibility level 1: Stable within a major release for a minimum of 12 months or 3 minor releases (whichever is longer).", } @@ -562,6 +582,7 @@ var map_ClusterVersionSpec = map[string]string{ "desiredUpdate": "desiredUpdate is an optional field that indicates the desired value of the cluster version. Setting this value will trigger an upgrade (if the current version does not match the desired version). The set of recommended update values is listed as part of available updates in status, and setting values outside that range may cause the upgrade to fail. You may specify the version field without setting image if an update exists with that version in the availableUpdates or history.\n\nIf an upgrade fails the operator will halt and report status about the failing component. Setting the desired update value back to the previous version will cause a rollback to be attempted. Not all rollbacks will succeed.", "upstream": "upstream may be used to specify the preferred update server. By default it will use the appropriate update server for the cluster and region.", "channel": "channel is an identifier for explicitly requesting that a non-default set of updates be applied to this cluster. The default channel will be contain stable updates that are appropriate for production clusters.", + "capabilities": "capabilities configures the installation of optional, core cluster components. A null value here is identical to an empty object; see the child properties for default semantics.", "overrides": "overrides is list of overides for components that are managed by cluster version operator. Marking a component unmanaged will prevent the operator from creating or updating the object.", } @@ -575,6 +596,7 @@ var map_ClusterVersionStatus = map[string]string{ "history": "history contains a list of the most recent versions applied to the cluster. This value may be empty during cluster startup, and then will be updated when a new update is being applied. The newest update is first in the list and it is ordered by recency. Updates in the history have state Completed if the rollout completed - if an update was failing or halfway applied the state will be Partial. Only a limited amount of update history is preserved.", "observedGeneration": "observedGeneration reports which version of the spec is being synced. If this value is not equal to metadata.generation, then the desired and conditions fields may represent a previous version.", "versionHash": "versionHash is a fingerprint of the content that the cluster will be updated with. It is used by the operator to avoid unnecessary work and is for internal use only.", + "capabilities": "capabilities describes the state of optional, core cluster components.", "conditions": "conditions provides information about the cluster version. The condition \"Available\" is set to true if the desiredUpdate has been reached. The condition \"Progressing\" is set to true if an update is being applied. The condition \"Degraded\" is set to true if an update is currently blocked by a temporary or permanent error. Conditions are only valid for the current desiredUpdate when metadata.generation is equal to status.generation.", "availableUpdates": "availableUpdates contains updates recommended for this cluster. Updates which appear in conditionalUpdates but not in availableUpdates may expose this cluster to known issues. This list may be empty if no updates are recommended, if the update service is unavailable, or if an invalid channel has been specified.", "conditionalUpdates": "conditionalUpdates contains the list of updates that may be recommended for this cluster if it meets specific required conditions. Consumers interested in the set of updates that are actually recommended for this cluster should use availableUpdates. This list may be empty if no updates are recommended, if the update service is unavailable, or if an empty or invalid channel has been specified.",