Bug 1777144: packageserver update fails to adopt APIService #1171
Conversation
openshift-ci-robot
added
the
bugzilla/valid-bug
|
@exdx: This pull request references Bugzilla bug 1777144, 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. 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. |
openshift-ci-robot
added
the
size/S
openshift-ci-robot
added
size/M
openshift-ci-robot
added
size/L
|
Nice work on this @exdx , could you add a rewrite the commit message to include:
|
|
@exdx could you also merge your commits into a single commit. |
|
@exdx I saw that the PR description included:
Were you able to test this and prove that the proposed solution will fix the bug? |
|
Thanks for the review @awgreene! Will update to incorporate your great feedback. |
exdx
changed the title
|
@exdx: This pull request references Bugzilla bug 1777144, which is valid. 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. |
exdx
force-pushed
the
bug-1777144
branch
2 times, most recently
from
72e1252
to
e3d7d26
Compare
exdx
force-pushed
the
bug-1777144
branch
2 times, most recently
from
f7d5f5d
to
ea8a632
Compare
exdx
force-pushed
the
bug-1777144
branch
2 times, most recently
from
ba409b9
to
77a5e9f
Compare
|
/retest |
1 similar comment
|
/retest |
openshift-ci-robot
removed
the
do-not-merge/hold
|
It looks like the packageserver owner labels to the api service should already be set when creating the APIService (we were not sure originally if they were being set correctly). That being said, the addition of the cleanup code should still be helpful in resolving the original bug on cluster upgrade? Also the bug was targeting a 4.2.z release for a 4.1 -> 4.2 upgrade, need to check that this all still applies. |
…e to adopt APIService status message. This is a result of ownership mismatches between the packageserver (owner) and the APIService (the owned object). This can be resolved by setting the actual k8s owner references or the label-based references that OLM uses. This commit ensures that ownership references for the APIService are cleaned up on OLM restart and that the olm.owner:packageserver label is set on the corresponding APIService object.
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
@exdx: This pull request references Bugzilla bug 1777144, which is valid. 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ecordell, exdx, 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 Please review the full test history for this PR and help us cut down flakes. |
3 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@exdx: All pull requests linked via external trackers have merged. Bugzilla bug 1777144 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.3 |
|
@exdx: new pull request created: #1270 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.2 |
|
@exdx: new pull request created: #1271 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:
This PR does 2 things to address the issue where updating the packageserver fails with a
unable to adopt APIServicemessage. One is add APIService to the cleanup owner references logic that OLM performs on startup to ensure owner references on APIService objects are correct.The other is adding verification on the labels of the APIService object to make sure that the labelolm.owner:packageserveris set.Motivation for the change:
OLM already checks several types for owner reference mismatches: mainly rbac types like roles and role bindings. Adding the check for APIService owner references will hopefully resolve the issue during cluster upgrade where the packageserver blocks upgrade because it is unable to set the ownership for its api service. The packageserver CSV fails to resolve with
message: unable to adopt APIServiceand this causes cluster upgrade to fail. By checking APIService references on startup this could potentially resolve the issue.Owner references are also established by labels on OLM objects, so adding a check forolm.owner:packageservershould also help.Reviewer Checklist
/docs