Skip to content
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

Jump to bottom

Adds bindings for spoke users on hub #111

Merged
merged 1 commit into from May 16, 2022
Merged

Adds bindings for spoke users on hub #111

merged 1 commit into from May 16, 2022

Conversation

vbnrh
Copy link
Member

@vbnrh vbnrh commented May 4, 2022

This commit adds changes that binds the hub's clusterrole to generated
spoke users.

This is done to fix a bug where the tokenagent on the spokeclusters are
unable to watch and make channges to mirrorpeer on the hub

Signed-off-by: Vineet Badrinath vineetbnath@gmail.com

@vbnrh vbnrh force-pushed the spoke-clusterrolebinding branch from b63a49c to bef2033 Compare May 4, 2022 13:04
@vbnrh vbnrh force-pushed the spoke-clusterrolebinding branch from bef2033 to 81a074d Compare May 4, 2022 13:06
@weirdwiz
Copy link
Contributor

weirdwiz commented May 5, 2022

LGTM, tested it out and it's working

@vbnrh vbnrh force-pushed the spoke-clusterrolebinding branch from 81a074d to 67c463d Compare May 5, 2022 05:47
@vbnrh vbnrh force-pushed the spoke-clusterrolebinding branch from 67c463d to 5f7c5c3 Compare May 6, 2022 11:25
@vbnrh vbnrh force-pushed the spoke-clusterrolebinding branch from 5f7c5c3 to 2282079 Compare May 9, 2022 10:32
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 11, 2022

@GowthamShanmugam: changing LGTM is restricted to collaborators

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.

@vbnrh vbnrh changed the title Adds bindings for spoke users on hub Bug 2082078: Adds bindings for spoke users on hub May 16, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 16, 2022

@vbnrh: This pull request references Bugzilla bug 2082078, which is valid.

No validations were run on this bug

Requesting review from QA contact:
/cc @keesturam

In response to this:

Bug 2082078: Adds bindings for spoke users on hub

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
Copy link
Contributor

openshift-ci bot commented May 16, 2022

@openshift-ci[bot]: GitHub didn't allow me to request PR reviews from the following users: keesturam.

Note that only red-hat-storage members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

@vbnrh: This pull request references Bugzilla bug 2082078, which is valid.

No validations were run on this bug

Requesting review from QA contact:
/cc @keesturam

In response to this:

Bug 2082078: Adds bindings for spoke users on hub

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.

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.

umangachapagain
umangachapagain requested changes May 16, 2022
View reviewed changes
controllers/mirrorpeer_controller.go Outdated Show resolved Hide resolved
controllers/mirrorpeer_controller.go Outdated Show resolved Hide resolved
controllers/mirrorpeer_controller.go Outdated Show resolved Hide resolved
@vbnrh vbnrh changed the title Bug 2082078: Adds bindings for spoke users on hub Adds bindings for spoke users on hub May 16, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 16, 2022

@vbnrh: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

In response to this:

Adds bindings for spoke users on hub

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.

@vbnrh vbnrh force-pushed the spoke-clusterrolebinding branch from 2282079 to 57b6383 Compare May 16, 2022 12:06
umangachapagain
umangachapagain requested changes May 16, 2022
View reviewed changes
controllers/mirrorpeer_controller.go Outdated
@@ -179,6 +183,12 @@ func (r *MirrorPeerReconciler) Reconcile(ctx context.Context, req ctrl.Request)
return ctrl.Result{}, err
}

err = r.createClusterRoleBindingsForSpoke(ctx, mirrorPeer)
if err != nil {
logger.Error(err, "Failed to create role bindings for spoke")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
logger.Error(err, "Failed to create role bindings for spoke")
logger.Error(err, "Failed to create cluster role bindings for spoke")

@vbnrh
Adds bindings for spoke users on hub
0f8a4e2 
This commit adds changes that binds the hub's clusterrole to generated
spoke users.

This is done to fix a bug where the tokenagent on the spokeclusters are
unable to watch and make channges to mirrorpeer on the hub

Signed-off-by: Vineet Badrinath <vineetbnath@gmail.com>
@vbnrh vbnrh force-pushed the spoke-clusterrolebinding branch from 57b6383 to 0f8a4e2 Compare May 16, 2022 12:12
@openshift-ci openshift-ci bot added the lgtm label May 16, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 16, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: GowthamShanmugam, umangachapagain, vbnrh

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:
  • OWNERS [umangachapagain,vbnrh]

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

@vbnrh
Copy link
Member Author

vbnrh commented May 16, 2022

/cherry-pick release-4.10

@openshift-cherrypick-robot
Copy link

@vbnrh: once the present PR merges, I will cherry-pick it on top of release-4.10 in a new PR and assign it to you.

In response to this:

/cherry-pick release-4.10

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.

@vbnrh
Copy link
Member Author

vbnrh commented May 16, 2022

/test integration-test

@openshift-merge-robot openshift-merge-robot merged commit a151584 into red-hat-storage:main May 16, 2022
8 checks passed
@openshift-cherrypick-robot
Copy link

@vbnrh: #111 failed to apply on top of branch "release-4.10":

Applying: Adds bindings for spoke users on hub
Using index info to reconstruct a base tree...
M	bundle/manifests/odf-multicluster-orchestrator.clusterserviceversion.yaml
M	config/rbac/role.yaml
M	controllers/mirrorpeer_controller.go
A	controllers/utils/slice.go
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): controllers/utils/slice.go deleted in HEAD and modified in Adds bindings for spoke users on hub. Version Adds bindings for spoke users on hub of controllers/utils/slice.go left in tree.
Auto-merging controllers/mirrorpeer_controller.go
CONFLICT (content): Merge conflict in controllers/mirrorpeer_controller.go
Auto-merging config/rbac/role.yaml
Auto-merging bundle/manifests/odf-multicluster-orchestrator.clusterserviceversion.yaml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Adds bindings for spoke users on hub
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".

In response to this:

/cherry-pick release-4.10

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-cherrypick-robot
Copy link

@agarwal-mudit: new pull request could not be created: failed to create pull request against red-hat-storage/odf-multicluster-orchestrator#release-4.11 from head openshift-cherrypick-robot:cherry-pick-111-to-release-4.11: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"No commits between red-hat-storage:release-4.11 and openshift-cherrypick-robot:cherry-pick-111-to-release-4.11"}],"documentation_url":"https://docs.github.com/rest/reference/pulls#create-a-pull-request"}

In response to this:

/cherry-pick release-4.11

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 openshift-ci bot mentioned this pull request May 18, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@umangachapagain umangachapagain umangachapagain approved these changes

@GowthamShanmugam GowthamShanmugam GowthamShanmugam approved these changes

Assignees

@umangachapagain umangachapagain

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@vbnrh @weirdwiz @openshift-cherrypick-robot @umangachapagain @GowthamShanmugam @openshift-merge-robot