Bug 1834936: fix(resolver): don't pick operator from same package when resolve requiredAPI #1521
Conversation
openshift-ci-robot
added
the
do-not-merge/work-in-progress
dinhxuanvu
changed the title
openshift-ci-robot
added
bugzilla/severity-urgent
|
@dinhxuanvu: This pull request references Bugzilla bug 1834936, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
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. |
dinhxuanvu
force-pushed
the
required-api-package
branch
5 times, most recently
from
b5df440 to
d5fe81d
Compare
dinhxuanvu
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
/retest |
There was a problem hiding this comment.
Nice work Vu! I had some review comments for the e2e test. With the plan that we would have all our e2e tests in ginkgo, I have provided some pointers for the same. It could go in as part of a separate PR if this is a priority bug fix. I want everyone in the team to be familiar with the new way of writing tests. Feel free to reach out if you have any questions. I am happy to pair program if it helps :)
|
/retest |
|
@dinhxuanvu: This pull request references Bugzilla bug 1834936, which is valid. 3 validation(s) were run on this bug
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. |
dinhxuanvu
force-pushed
the
required-api-package
branch
5 times, most recently
from
bf8a7f8 to
0b033dc
Compare
…uiredAPI When resolver looks for an operator that provides requiredAPIs, it doesn't exclude operators from the same package that the original operator that requires those APIs. As a result, if there is an existing version of that operator now provides those requiredAPIs (due to API ownership transfer), resolver will select an older or newer version of the same operator to fulfill that requiredAPIs which leads to API ownership conflicts on providedAPIs. The resolver will look up all latest channel entries that provides the necessary requiredAPIs and filter out those from the same package before go on with the operator selection. Signed-off-by: Vu Dinh <[email protected]>
1. Rename fake_registry_client to fake_registry_interface for registry client interface. 2. Generate fake_registry_client for the actual reigstry client only. Signed-off-by: Vu Dinh <[email protected]>
Signed-off-by: Vu Dinh <[email protected]>
Signed-off-by: Vu Dinh <[email protected]>
dinhxuanvu
force-pushed
the
required-api-package
branch
from
0b033dc to
9258f5f
Compare
There was a problem hiding this comment.
Looking good so far. Added some comments.
It may help to take a look at the embedding section of effective go to help guide these changes.
Signed-off-by: Vu Dinh <[email protected]>
|
/retest |
1 similar comment
|
/retest |
openshift-ci-robot
added
the
approved
dinhxuanvu
force-pushed
the
required-api-package
branch
from
928b12f to
0b02331
Compare
Signed-off-by: Vu Dinh <[email protected]>
dinhxuanvu
force-pushed
the
required-api-package
branch
from
0b02331 to
57b5881
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dinhxuanvu, ecordell 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 |
|
/lgtm |
|
/test e2e-gcp |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest |
|
@dinhxuanvu: All pull requests linked via external trackers have merged: operator-framework/operator-lifecycle-manager#1521. Bugzilla bug 1834936 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. |
|
/cherry-pick release-4.4 |
|
@dinhxuanvu: #1521 failed to apply on top of branch "release-4.4": 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. |
When resolver looks for an operator that provides requiredAPIs, it
doesn't exclude operators from the same package that the original
operator that requires those APIs. As a result, if there is an
existing version of that operator now provides those requiredAPIs
(due to API ownership transfer), resolver will select an older or
newer version of the same operator to fulfill that requiredAPIs
which leads to API ownership conflicts on providedAPIs.
The resolver will look up all latest channel entries that provides
the necessary requiredAPIs and filter out those from the same package
before go on with the operator selection.
Signed-off-by: Vu Dinh [email protected]
Description of the change:
Motivation for the change:
Note to reviewer:
This PR is designed to be backport-friendly as it is planned to be backported all the way down to 4.2. As a result, there are significant amount of code which is here solely for backport reason. After the backport is done, there will be follow-up PR(s) to update OLM and registry and clean up the codebase in the master.
Reviewer Checklist
/docs