Support semver ranges of versions to skip in the head of a channel #834
Conversation
openshift-ci-robot
added
approved
ecordell
force-pushed
the
semver-range
branch
2 times, most recently
from
25fa100 to
e3ed0a2
Compare
|
/retest |
2 similar comments
|
/retest |
|
/retest |
the blang implementation supports range queries
ecordell
force-pushed
the
semver-range
branch
5 times, most recently
from
ec0f6b9 to
943349a
Compare
explicitly states that a range of existing operators can update directly this is to support z-stream releases for operators, where there is a reasonable degree of confidence that it's safe to skip updates, and where it is difficult to generate the graph ahead of time
| ) | ||
|
|
||
| const SkipPackageAnnotationKey = "olm.skipRange" |
| @@ -207,6 +211,7 @@ func crd(key opregistry.APIKey) *v1beta1.CustomResourceDefinition { | |||
| } | |||
|
|
|||
| func u(object runtime.Object) *unstructured.Unstructured { | |||
| fmt.Println(object) | |||
There was a problem hiding this comment.
oops! just for tests though
| } | ||
|
|
||
| for key, source := range q.sources { | ||
| bundle, err := source.GetReplacementBundleInPackageChannel(context.TODO(), bundleName, pkgName, channelName) | ||
| if err == nil { | ||
| bundle, err := q.findChannelHead(currentVersion, pkgName, channelName, source) |
There was a problem hiding this comment.
Should we not return the bundle if it's version matches currentVersion?
| @@ -77,7 +79,7 @@ var ( | |||
| }, | |||
| Spec: v1alpha1.ClusterServiceVersionSpec{ | |||
| Replaces: "", | |||
| Version: *semver.New("0.1.0"), | |||
| Version: version.OperatorVersion{semver.MustParse("0.1.0")}, | |||
There was a problem hiding this comment.
can we have a wrapper version.MustParse(...) function?
| mainNamedStrategy := newNginxInstallStrategy(genName("dep-"), nil, nil) | ||
| mainCSV := newCSV(mainPackageStable, testNamespace, "", semver.MustParse("0.1.0-1556661347"), nil, nil, mainNamedStrategy) | ||
| updatedCSV := newCSV(updatedPackageStable, testNamespace, "", semver.MustParse("0.1.0-1556661832"), nil, nil, mainNamedStrategy) | ||
| updatedCSV.SetAnnotations(map[string]string{resolver.SkipPackageAnnotationKey: ">=0.1.0-1556661347 <0.1.0-1556661832"}) |
There was a problem hiding this comment.
Is 0.1.0-1556661832 exclusive to prevent it from being picked up as a replacement after it's installed?
| } | ||
|
|
||
| for key, source := range q.sources { | ||
| bundle, err := source.GetReplacementBundleInPackageChannel(context.TODO(), bundleName, pkgName, channelName) | ||
| if err == nil { | ||
| bundle, err := q.findChannelHead(currentVersion, pkgName, channelName, source) |
There was a problem hiding this comment.
Since we're iterating over a map, which is non-deterministic, if we have multiple catalogs with the same package/channel and CSVs at head with olm.skipRanges that include currentVersion, is it possible that we can have non-deterministic upgrades.
Could we also get false "upgrade available" detections if the range is inclusive of head?
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ecordell, njhale 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 |
|
/retest |
1 similar comment
|
/retest |
Define a new annotation for CSVs:
where
<semver range>has the semver range format supported by the semver lib we're using.When searching catalogs for updates, if the head of a channel has this annotation and the currently installed operator has a
versionfield that falls in the range, we will update to the latest entry in the channel.The order of precedence is:
sourceNameon the subscription, if the other criteria for skipping are metreplacesthe current one, in the source specified bysourceNamereplacesthe current one in any source visible to the subscription(haven't run the e2e tests locally yet b/c of my current internet connection)