Introduce OLMConfig CRD #198
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
awgreene
force-pushed
the
olm-config
branch
3 times, most recently
from
e2ef0ee to
c8315c8
Compare
pkg/operators/v1/olmconfig_types.go
Outdated
| // OperatorGroup that targets all namespaces. | ||
| // When reenabled, OLM will recreate the "Copied CSVs" for each | ||
| // cluster scoped operator. | ||
| DisableCopiedCSVs bool `json:"disableCopiedCSVs,omitempty"` |
There was a problem hiding this comment.
You may need to make this a pointer to a bool to avoid disableCopiedCSVs: false being implicitly specified (or printed when firing off kubectl get olmconfigs <...> -o yaml).
There was a problem hiding this comment.
Ha! Actually landed on this change while consuming the API. Done!
There was a problem hiding this comment.
This was marked as resolved, but I noticed that its not (or no longer) a pointer to a bool? (I agree that it should be a pointer to a bool)
awgreene
force-pushed
the
olm-config
branch
2 times, most recently
from
5782b40 to
cfec086
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: awgreene, kevinrizza 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:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
awgreene
force-pushed
the
olm-config
branch
4 times, most recently
from
794bf4d to
1450518
Compare
pkg/operators/v1/olmconfig_types.go
Outdated
| ) | ||
|
|
||
| const ( | ||
| DisableCopiedCSVsConditionType = "DisableCopiedCSVsConfiguration" |
There was a problem hiding this comment.
Should this be past tense so that it reads a little more like some state has (or hasn't) been achieved? Not sure if this was captured in the EP. If so, disregard. E.g.:
conditions:
- type: DisabledCopiedCSVs
status: TrueOr:
conditions:
- type: DisabledCopiedCSVs
status: False
reason: DeleteCSVFailure
message: 'delete csv "foo" in namespace "bar": not permitted'There was a problem hiding this comment.
This seems fine and was not captured in the EP
There was a problem hiding this comment.
Addressed in the latest version of the PR. Note that it isn't possible to discern the state of the condition without both the status and reason:
status is false because copied csvs are enabled:
conditions:
- type: DisabledCopiedCSVs
status: False
reason: CopiedCSVs enabled
message: CSVs are being copiedvs.
status is false because not all copied csvs were deleted:
conditions:
- type: DisabledCopiedCSVs
status: False
reason: DeleteCSVFailure
message: 'delete csv "foo" in namespace "bar": not permitted'Not really an issue, but something worth calling out in the eventual docs.
|
/lgtm |
No description provided.