Skip to content

resolver: remove legacy support for fallback parsing of CSVs#2291

Merged
openshift-merge-robot merged 1 commit intooperator-framework:masterfrom
joelanford:bz-1969902
Jul 27, 2021
Merged

resolver: remove legacy support for fallback parsing of CSVs#2291
openshift-merge-robot merged 1 commit intooperator-framework:masterfrom
joelanford:bz-1969902

Conversation

@joelanford
Copy link
Copy Markdown
Member

@joelanford joelanford commented Jul 27, 2021

Description of the change:
This commit removes support for the legacy CSV parsing fallback, which
means that only first class fields in the GRPC API will be used during
resolutions.

Effectively reverts #1194

Motivation for the change:
When no APIs or properties are present on bundles, the resolver
currently falls back to a legacy mode where that information is parsed
from CSVs present in the index.

This legacy fallback method causes the resolver to incorrectly identify
multiple heads for a channel when:
a) The index contains CSVs only on channel head bundles, and
b) A package contains two channels, where one channel contains the head
of the other channel and there is at least one other non-channel-head node between
these channel head nodes in that channel.
c) The bundles have no properties, provided APIs, or required APIs

Conditions a) and b) are extremely prevalent, so this bug will often be
encountered whenever c) itself is true.

Reviewer Checklist

  • Implementation matches the proposed design, or proposal is updated to match implementation
  • Sufficient unit test coverage
  • Sufficient end-to-end test coverage
  • Docs updated or added to /doc
  • Commit messages sensible and descriptive

@openshift-ci openshift-ci Bot requested review from exdx and timflannagan July 27, 2021 00:34
When no APIs or properties are present on bundles, the resolver
currently falls back to a legacy mode where that information is parsed
from CSVs present in the index.

This legacy fallback method causes the resolver to incorrectly identify
multiple heads for a channel when:
a) The index contains CSVs only on channel head bundles, and
b) A package contains two channels, where one channel contains the head
   of the other channel and there is at least one other node between
   these channel head nodes in that channel.
c) The bundles have no properties, provided APIs, or required APIs

Conditions a) and b) are extremely prevalent, so this bug will often be
encountered whenever c) itself is true.

This commit removes support for the legacy CSV parsing fallback, which
means that only first class fields in the GRPC API will be used during
resolutions.

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
Copy link
Copy Markdown
Member

@ecordell ecordell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 27, 2021
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Jul 27, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ecordell, joelanford

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 27, 2021
@ecordell
Copy link
Copy Markdown
Member

/hold

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 27, 2021
@openshift-merge-robot openshift-merge-robot merged commit 455975c into operator-framework:master Jul 27, 2021
@joelanford joelanford deleted the bz-1969902 branch July 27, 2021 16:36
@timflannagan
Copy link
Copy Markdown
Member

/cherry-pick release-4.7

@openshift-cherrypick-robot
Copy link
Copy Markdown

@timflannagan: #2291 failed to apply on top of branch "release-4.7":

Applying: resolver: remove legacy support for fallback parsing of CSVs
Using index info to reconstruct a base tree...
M	pkg/controller/registry/resolver/operators.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/controller/registry/resolver/operators.go
CONFLICT (content): Merge conflict in pkg/controller/registry/resolver/operators.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 resolver: remove legacy support for fallback parsing of CSVs
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherry-pick release-4.7

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants