Bug 2076187: Identify fail forward in csvSources #2743
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: awgreene 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
| @@ -196,80 +196,6 @@ func WithInstalledCSV(sub *v1alpha1.Subscription, csvName string) *v1alpha1.Subs | |||
| return sub | |||
| } | |||
|
|
|||
| func TestSolveOperators_WithFailForward(t *testing.T) { | |||
There was a problem hiding this comment.
Removed since the changes are not done on the resolver level anymore.
awgreene
force-pushed
the
update-csvSource
branch
6 times, most recently
from
92e64d7 to
dba1434
Compare
awgreene
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
awgreene
force-pushed
the
update-csvSource
branch
from
dba1434 to
e9fa146
Compare
awgreene
changed the title
awgreene
force-pushed
the
update-csvSource
branch
from
e9fa146 to
4a20ee3
Compare
This commit undos changes to the Resolve and ResolveSteps methods and updats the csvSourceProvider to infer whether or not fail forward is enabled in a namespace. Signed-off-by: Alexander Greene <[email protected]>
awgreene
force-pushed
the
update-csvSource
branch
from
4a20ee3 to
a4b2cf0
Compare
There was a problem hiding this comment.
Great quality of life improvement here for the resolver as we don't have to continuously add the "fail forward" boolean for resolution. Eventually we'll need to remove the concept of CSV's from the resolver package entirely as a result of the breakout but until then this look
good. One small non-blocking nit.
/lgtm
| // IsFailForwardEnabled takes a namespaced operatorGroup lister and returns | ||
| // True if an operatorGroup exists in the namespace and its upgradeStrategy | ||
| // is set to UnsafeFailForward and false otherwise. An error is returned if | ||
| // an more than one operatorGroup exists in the namespace. | ||
| // No error is returned if no OperatorGroups are found to keep the resolver | ||
| // backwards compatible. |
There was a problem hiding this comment.
nit:
| // IsFailForwardEnabled takes a namespaced operatorGroup lister and returns | |
| // True if an operatorGroup exists in the namespace and its upgradeStrategy | |
| // is set to UnsafeFailForward and false otherwise. An error is returned if | |
| // an more than one operatorGroup exists in the namespace. | |
| // No error is returned if no OperatorGroups are found to keep the resolver | |
| // backwards compatible. | |
| // IsFailForwardEnabled takes a namespaced operatorGroup lister and returns | |
| // True if an operatorGroup exists in the namespace and its upgradeStrategy | |
| // is set to UnsafeFailForward and false otherwise. An error is returned if | |
| // more than one operatorGroup exists in the namespace. No error is returned | |
| // if no OperatorGroups are found to keep the resolver backwards compatible. |
awgreene
changed the title
|
@awgreene: All pull requests linked via external trackers have merged: Bugzilla bug 2076187 has been moved to the MODIFIED state. In response to this:
Instructions 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. |
Description of the change:
Motivation for the change:
Reviewer Checklist
/doc[FLAKE]are truly flaky[FLAKE]tag are no longer flaky