config/v1/types_cluster_version: Add capabilties properties #1106
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wking
force-pushed
the
capabilities
branch
4 times, most recently
from
90cc976 to
2cdba20
Compare
kirankt
reviewed
Feb 1, 2022
kirankt
left a comment
kirankt
left a comment
There was a problem hiding this comment.
/lgtm
Thanks for the PR. Looking forward to using it in the installer.
bparees
reviewed
Feb 1, 2022
Member
Author
|
@openshift/api-reviewers, can you take a look? With Ben satisfied, I don't think there's anyone else in particular we're waiting on. |
deads2k
reviewed
Feb 3, 2022
deads2k
reviewed
Feb 3, 2022
deads2k
reviewed
Feb 3, 2022
deads2k
reviewed
Feb 3, 2022
deads2k
reviewed
Feb 3, 2022
deads2k
reviewed
Feb 3, 2022
deads2k
reviewed
Feb 4, 2022
deads2k
reviewed
Feb 4, 2022
deads2k
reviewed
Feb 4, 2022
deads2k
reviewed
Feb 4, 2022
deads2k
reviewed
Feb 4, 2022
wking
force-pushed
the
capabilities
branch
4 times, most recently
from
06f44f9 to
224e90e
Compare
wking
commented
Feb 4, 2022
wking
added a commit
to wking/openshift-api
that referenced
this pull request
Feb 4, 2022
Roughly as described in [enhancement]. After discussion with Ben and
David, we've made the following changes:
# status
## enabledCapabilities
This had been include in [enhancement], but David prefers
enabledCapabilities [statusPropertyNames] and Ben was fine with that.
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"`
}
Hmm...
* Status uses knownCapabilities and
I've left inclusionDefault out of the
status structure, because I'm using an explicit, exhaustive list for
include and exclude there. The exhaustive excluded list will provide
a convenient set 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 list, 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.
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
[statusPropertyNames]: openshift#1106 (comment)
Contributor
|
@wking: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Contributor
|
/hold cancel /lgtm |
openshift-ci
bot
added
lgtm
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bparees, deads2k, kirankt, wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Contributor
|
this is being consumed by a FF team, so overriding the no-FF labels /label qe-approved |
openshift-ci
bot
added
qe-approved
wking
added a commit
to wking/openshift-enhancements
that referenced
this pull request
May 5, 2022
Lively discussion in [1] lead to a bunch of changes, as documented in the commit message [2]. Bring those back to the enhancemnt to reduce enhancement-vs-reality confusion. [1]: openshift/api#1106 [2]: openshift/api@5b82635
wking
added a commit
to wking/openshift-enhancements
that referenced
this pull request
May 5, 2022
Lively discussion in [1] lead to a bunch of changes, as documented in the commit message [2]. Bring those back to the enhancement to reduce enhancement-vs-reality confusion. [1]: openshift/api#1106 [2]: openshift/api@5b82635
wking
added a commit
to wking/openshift-enhancements
that referenced
this pull request
May 5, 2022
Lively discussion in [1] lead to a bunch of changes, as documented in the commit message [2]. Bring those back to the enhancement to reduce enhancement-vs-reality confusion. [1]: openshift/api#1106 [2]: openshift/api@5b82635
wking
added a commit
to wking/openshift-enhancements
that referenced
this pull request
May 5, 2022
Lively discussion in [1] lead to a bunch of changes, as documented in the commit message [2]. Bring those back to the enhancement to reduce enhancement-vs-reality confusion. [1]: openshift/api#1106 [2]: openshift/api@5b82635
wking
added a commit
to wking/openshift-enhancements
that referenced
this pull request
May 5, 2022
Lively discussion in [1] lead to a bunch of changes, as documented in the commit message [2]. Bring those back to the enhancement to reduce enhancement-vs-reality confusion. [1]: openshift/api#1106 [2]: openshift/api@5b82635
wking
added a commit
to wking/openshift-enhancements
that referenced
this pull request
May 5, 2022
Lively discussion in [1] lead to a bunch of changes, as documented in the commit message [2]. Bring those back to the enhancement to reduce enhancement-vs-reality confusion. [1]: openshift/api#1106 [2]: openshift/api@5b82635
wking
added a commit
to wking/openshift-enhancements
that referenced
this pull request
May 5, 2022
Lively discussion in [1] lead to a bunch of changes, as documented in the commit message [2]. Bring those back to the enhancement to reduce enhancement-vs-reality confusion. [1]: openshift/api#1106 [2]: openshift/api@5b82635
wking
added a commit
to wking/openshift-enhancements
that referenced
this pull request
May 5, 2022
Lively discussion in [1] lead to a bunch of changes, as documented in the commit message [2]. Bring those back to the enhancement to reduce enhancement-vs-reality confusion. The title change and similar metadata updates satisfy the linter [3]: enhancements/installer/component-selection.md: the title component-selection-during-install and the file base name component-selection must match enhancements/installer/component-selection.md: api-approvers must have at least one valid github id [1]: openshift/api#1106 [2]: openshift/api@5b82635 [3]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_enhancements/1109/pull-ci-openshift-enhancements-master-markdownlint/1522345900326785024#1:build-log.txt%3A47-48
wking
added a commit
to wking/openshift-enhancements
that referenced
this pull request
May 6, 2022
Lively discussion in [1] lead to a bunch of changes, as documented in the commit message [2]. Bring those back to the enhancement to reduce enhancement-vs-reality confusion. The title change and similar metadata updates satisfy the linter [3]: enhancements/installer/component-selection.md: the title component-selection-during-install and the file base name component-selection must match enhancements/installer/component-selection.md: api-approvers must have at least one valid github id [1]: openshift/api#1106 [2]: openshift/api@5b82635 [3]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_enhancements/1109/pull-ci-openshift-enhancements-master-markdownlint/1522345900326785024#1:build-log.txt%3A47-48
wking
added a commit
to wking/openshift-enhancements
that referenced
this pull request
May 6, 2022
Lively discussion in [1] lead to a bunch of changes, as documented in the commit message [2]. Bring those back to the enhancement to reduce enhancement-vs-reality confusion. The title change and similar metadata updates satisfy the linter [3]: enhancements/installer/component-selection.md: the title component-selection-during-install and the file base name component-selection must match enhancements/installer/component-selection.md: api-approvers must have at least one valid github id [1]: openshift/api#1106 [2]: openshift/api@5b82635 [3]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_enhancements/1109/pull-ci-openshift-enhancements-master-markdownlint/1522345900326785024#1:build-log.txt%3A47-48
wking
added a commit
to wking/openshift-enhancements
that referenced
this pull request
May 6, 2022
Lively discussion in [1] lead to a bunch of changes, as documented in the commit message [2]. Bring those back to the enhancement to reduce enhancement-vs-reality confusion. The title change and similar metadata updates satisfy the linter [3]: enhancements/installer/component-selection.md: the title component-selection-during-install and the file base name component-selection must match enhancements/installer/component-selection.md: api-approvers must have at least one valid github id [1]: openshift/api#1106 [2]: openshift/api@5b82635 [3]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_enhancements/1109/pull-ci-openshift-enhancements-master-markdownlint/1522345900326785024#1:build-log.txt%3A47-48
wking
added a commit
to wking/openshift-enhancements
that referenced
this pull request
May 10, 2022
Lively discussion in [1] lead to a bunch of changes, as documented in the commit message [2]. Bring those back to the enhancement to reduce enhancement-vs-reality confusion. The title change and similar metadata updates satisfy the linter [3]: enhancements/installer/component-selection.md: the title component-selection-during-install and the file base name component-selection must match enhancements/installer/component-selection.md: api-approvers must have at least one valid github id [1]: openshift/api#1106 [2]: openshift/api@5b82635 [3]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_enhancements/1109/pull-ci-openshift-enhancements-master-markdownlint/1522345900326785024#1:build-log.txt%3A47-48
wking
added a commit
to wking/openshift-enhancements
that referenced
this pull request
May 10, 2022
Lively discussion in [1] lead to a bunch of changes, as documented in the commit message [2]. Bring those back to the enhancement to reduce enhancement-vs-reality confusion. The title change and similar metadata updates satisfy the linter [3]: enhancements/installer/component-selection.md: the title component-selection-during-install and the file base name component-selection must match enhancements/installer/component-selection.md: api-approvers must have at least one valid github id [1]: openshift/api#1106 [2]: openshift/api@5b82635 [3]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_enhancements/1109/pull-ci-openshift-enhancements-master-markdownlint/1522345900326785024#1:build-log.txt%3A47-48
wking
added a commit
to wking/openshift-enhancements
that referenced
this pull request
May 10, 2022
Lively discussion in [1] lead to a bunch of changes, as documented in the commit message [2]. Bring those back to the enhancement to reduce enhancement-vs-reality confusion. The title change and similar metadata updates satisfy the linter [3]: enhancements/installer/component-selection.md: the title component-selection-during-install and the file base name component-selection must match enhancements/installer/component-selection.md: api-approvers must have at least one valid github id [1]: openshift/api#1106 [2]: openshift/api@5b82635 [3]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_enhancements/1109/pull-ci-openshift-enhancements-master-markdownlint/1522345900326785024#1:build-log.txt%3A47-48
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Roughly as described in the enhancement. I've left
inclusionDefaultout of thestatusstructure, because I'm using an explicit, exhaustive list for include and exclude there. The exhaustive excluded list will provide a convenient set 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 list, 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.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 useClusterOperatorStatusCondition, and the new condition type would not be something that makes sense for ClusterOperator.