Skip to content

add role for quota subtrees #475

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.

Sign up for GitHub

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
Merged
merged 3 commits into from
Jul 21, 2023
Merged

add role for quota subtrees #475

merged 3 commits into from
Jul 21, 2023

Conversation

asm582
Copy link
Member

@asm582 asm582 commented Jul 12, 2023

No description provided.

@asm582 asm582 mentioned this pull request Jul 13, 2023
Closed
@jbusche
Copy link
Contributor

jbusche commented Jul 14, 2023

@asm582 I tried this with your helm command and it didn't fully work. I had to add this under resources:

  - quotasubtrees

So the whole system:controller:xqueuejob-controller clusterrole apigroup/resources looked like this:

rules:
- apiGroups:
  - mcad.ibm.com
  - ibm.com
  resources:
  - quotasubtrees
  - xqueuejobs
  - queuejobs
  - schedulingspecs
  - appwrappers
  - appwrappers/finalizers
  - appwrappers/status

@sutaakar
Copy link
Contributor

Just a note - roles need to be also adjusted in operator kubebuilder - https://github.com/project-codeflare/codeflare-operator/blob/9c269d0881a0a37166c120963d747b543413df4e/controllers/mcad_controller.go#L106 (and regenerate config files).

@asm582
Copy link
Member Author

asm582 commented Jul 17, 2023

@jbusche @sutaakar please feel free to close this PR and submit new one with working changes or you can build on this PR, thanks for your comments

z103cb
z103cb previously approved these changes Jul 17, 2023
View reviewed changes
@jbusche jbusche mentioned this pull request Jul 18, 2023
Closed
@asm582
Copy link
Member Author

asm582 commented Jul 19, 2023

@jbusche @sutaakar please feel free to close this PR and submit new one with working changes or you can build on this PR, thanks for your comments

@sutaakar kind nudge to review this PR :)

z103cb
z103cb previously approved these changes Jul 19, 2023
View reviewed changes
Copy link
Contributor

@z103cb z103cb left a comment

Choose a reason for hiding this comment

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

Small improvement on the golang code, otherwise LGTM>

@jbusche jbusche mentioned this pull request Jul 19, 2023
Closed
@sutaakar
Copy link
Contributor

@asm582 Role adjustment looks good, will approve once you revert the other change (as you mentioned in your comment)

@asm582
Copy link
Member Author

asm582 commented Jul 20, 2023

@asm582 Role adjustment looks good, will approve once you revert the other change (as you mentioned in your comment)

@sutaakar Sure reverted code changes

@asm582 asm582 requested a review from sutaakar July 20, 2023 14:29
@asm582 asm582 merged commit 2f140ed into project-codeflare:main Jul 21, 2023
@jbusche jbusche mentioned this pull request Aug 17, 2023
Closed
4 tasks
@jbusche jbusche mentioned this pull request Sep 12, 2023
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sutaakar sutaakar sutaakar approved these changes

@z103cb z103cb z103cb approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@asm582 @jbusche @sutaakar @z103cb