New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reverting Noobaa's SCC priority #591
Reverting Noobaa's SCC priority #591
Conversation
Hi @jackyalbo. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
/ok-to-test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jackyalbo, a few points:
-
I don't fully understand the description of the PR / the commit message yet: The bug referenced (https://bugzilla.redhat.com/show_bug.cgi?id=1850148) is an openshift Bug and has nothing in our code to revert.
-
Instead, the SCC was introduced in PR Creating a new noobaa SCC #511, related to BZ https://bugzilla.redhat.com/show_bug.cgi?id=1804168 and as far as I can tell, this is not a complete revert of that patch, but just removes two lines.
-
Can you explain a little more what the "unexpected behavior" is that leads to this?
/retest |
Hey @obnoxxx I updated my PR explanation, hope it makes things clear: |
29cab0d
to
79394aa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jackyalbo - thanks for following up!
I would like to go one more round:
Thanks for providing some context in the comment and in the PR description.
But could you please update the commit message with information what the problem is that is being fixed (and maybe the BZ link)?
And, more concretely, just giving the bz is a step in the right direction, but I would prefer to have a short problem description in the commit msg itself so that people can later understand why this change was done by just looking at the git log. E.g. the sentence from the BZ looks like a good start: "Because the NooBaa SCC's priority is set to be higher than the anyuid's, it results in loss of the default root access on all pods created in the openshift-storage
namespace."
==> Please update the commit message and force-push, then we'll be good!
Because the NooBaa SCC's priority is set to be higher than the anyuid's, it results in loss of the default root access on all pods created in the openshift-storage namespace. More info on the issue can be found here: BZ https://bugzilla.redhat.com/show_bug.cgi?id=1851697 Due to this issue we will now remove the SCC priority until we have better understanding why it effect all pods when suppose to effect only noobaa's Signed-off-by: jackyalbo jalbo@redhat.com
79394aa
to
14b7826
Compare
@obnoxxx commit was updated as well, with your proposed format |
@jackyalbo: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
thanks for the update!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jackyalbo, obnoxxx 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 |
/test ocs-operator-e2e-aws |
/retest Please review the full test history for this PR and help us cut down flakes. |
/cherry-pick release-4.5 |
@obnoxxx: new pull request created: #605 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. |
Changing the priority leads to unexpected behaviour - so partially reverting
BZ https://bugzilla.redhat.com/show_bug.cgi?id=1851697
CI Failure issue: https://bugzilla.redhat.com/show_bug.cgi?id=1850148
Signed-off-by: jackyalbo jalbo@redhat.com