You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by GitBox <gi...@apache.org> on 2022/12/22 10:15:54 UTC
[GitHub] [yunikorn-k8shim] zhuqi-lucas opened a new pull request, #508: [YUNIKORN-1499] Fix PriorityClass resource access for yunikorn-admission-controller when deploying.
zhuqi-lucas opened a new pull request, #508:
URL: https://github.com/apache/yunikorn-k8shim/pull/508
### What is this PR for?
When deploying yunikorn-admission-controller with
yunikorn-admission-controller-rbac, it shows can't list and watch for PriorityClass in pod log.
### What type of PR is it?
* [ ] - Bug Fix
* [ ] - Improvement
* [ ] - Feature
* [ ] - Documentation
* [ ] - Hot Fix
* [ ] - Refactoring
### Todos
* [ ] - Task
### What is the Jira issue?
* Open an issue on Jira https://issues.apache.org/jira/browse/YUNIKORN/
* Put link here, and add [YUNIKORN-*Jira number*] in PR title, eg. `[YUNIKORN-2] Gang scheduling interface parameters`
### How should this be tested?
### Screenshots (if appropriate)
### Questions:
* [ ] - The licenses files need update.
* [ ] - There is breaking changes for older versions.
* [ ] - It needs documentation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] wilfred-s commented on a diff in pull request #508: [YUNIKORN-1499] Use fine-grained K8s access control in example deployments.
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on code in PR #508:
URL: https://github.com/apache/yunikorn-k8shim/pull/508#discussion_r1057970059
##########
deployments/scheduler/yunikorn-rbac.yaml:
##########
@@ -26,7 +66,49 @@ metadata:
subjects:
- kind: ServiceAccount
name: yunikorn-admin
+ namespace: yunikorn
+roleRef:
+ kind: ClusterRole
+ name: yunikorn-scheduler
+ apiGroup: rbac.authorization.k8s.io
+
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+ name: yunikorn-rbac-kube-scheduler
+subjects:
+ - kind: ServiceAccount
+ name: yunikorn-admin
+ namespace: yunikorn
+roleRef:
+ kind: ClusterRole
+ name: system:kube-scheduler
+ apiGroup: rbac.authorization.k8s.io
+
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+ name: yunikorn-rbac-volume-scheduler
+subjects:
+ - kind: ServiceAccount
+ name: yunikorn-admin
+ namespace: yunikorn
roleRef:
kind: ClusterRole
- name: cluster-admin
+ name: system:volume-scheduler
apiGroup: rbac.authorization.k8s.io
+
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+ name: yunikorn-rbac
+subjects:
+ - kind: ServiceAccount
+ name: yunikorn-admin
+roleRef:
+ kind: Role
+ name: yunikorn-scheduler
+ apiGroup: rbac.authorization.k8s.io
Review Comment:
please make sure that we we have. a newline at the end of the file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] wilfred-s commented on pull request #508: [YUNIKORN-1499] Use fine-grained K8s access control in example deployments.
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on PR #508:
URL: https://github.com/apache/yunikorn-k8shim/pull/508#issuecomment-1363602080
The helm charts have been updated via: apache/yunikorn-release#120
A copy of the charts is used during the e2e tests so without that change none of the e2e tests would work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] zhuqi-lucas commented on pull request #508: [YUNIKORN-1499] Fix PriorityClass resource access for yunikorn-admission-controller when deploying.
Posted by GitBox <gi...@apache.org>.
zhuqi-lucas commented on PR #508:
URL: https://github.com/apache/yunikorn-k8shim/pull/508#issuecomment-1363580380
Thank you, @craigcondit, for review, in the scheduler rbac.yaml , we already used the cluster-admin cluster role which will cover the PriorityClass access.
`apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: yunikorn-rbac
subjects:
- kind: ServiceAccount
name: yunikorn-admin
roleRef:
kind: ClusterRole
name: cluster-admin
apiGroup: rbac.authorization.k8s.io
`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] zhuqi-lucas commented on pull request #508: [YUNIKORN-1499] Fix PriorityClass resource access for yunikorn-admission-controller when deploying.
Posted by GitBox <gi...@apache.org>.
zhuqi-lucas commented on PR #508:
URL: https://github.com/apache/yunikorn-k8shim/pull/508#issuecomment-1363592180
> We should not be using the cluster admin role anywhere. That was dropped as part of the [YUNIKORN-997](https://issues.apache.org/jira/browse/YUNIKORN-997) in release 1.0. Looks like we forgot to merge those changes intothe non helm examples.
Thank you @wilfred-s for this reminder, i will cover this in this PR, and i will also create a new PR to check the helm chart priorityclass related things.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] wilfred-s closed pull request #508: [YUNIKORN-1499] Use fine-grained K8s access control in example deployments.
Posted by GitBox <gi...@apache.org>.
wilfred-s closed pull request #508: [YUNIKORN-1499] Use fine-grained K8s access control in example deployments.
URL: https://github.com/apache/yunikorn-k8shim/pull/508
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] wilfred-s commented on pull request #508: [YUNIKORN-1499] Fix PriorityClass resource access for yunikorn-admission-controller when deploying.
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on PR #508:
URL: https://github.com/apache/yunikorn-k8shim/pull/508#issuecomment-1363581519
We should not be using the cluster admin role anywhere. That was dropped as part of the [YUNIKORN-997](https://issues.apache.org/jira/browse/YUNIKORN-997) in release 1.0. Looks like we forgot to merge those changes intothe non helm examples.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] zhuqi-lucas commented on pull request #508: [YUNIKORN-1499] Use fine-grained K8s access control in example deployments.
Posted by GitBox <gi...@apache.org>.
zhuqi-lucas commented on PR #508:
URL: https://github.com/apache/yunikorn-k8shim/pull/508#issuecomment-1363608275
It makes sense to me @wilfred-s , so i will cover the fine-grained and priority in example deployments, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] codecov[bot] commented on pull request #508: [YUNIKORN-1499] Fix PriorityClass resource access for yunikorn-admission-controller when deploying.
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #508:
URL: https://github.com/apache/yunikorn-k8shim/pull/508#issuecomment-1362667956
# [Codecov](https://codecov.io/gh/apache/yunikorn-k8shim/pull/508?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#508](https://codecov.io/gh/apache/yunikorn-k8shim/pull/508?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f46d30c) into [master](https://codecov.io/gh/apache/yunikorn-k8shim/commit/8dad421916fc0693adf0ac002e15a2ea8ee84d17?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8dad421) will **decrease** coverage by `0.02%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #508 +/- ##
==========================================
- Coverage 69.41% 69.39% -0.03%
==========================================
Files 45 45
Lines 7714 7714
==========================================
- Hits 5355 5353 -2
- Misses 2162 2164 +2
Partials 197 197
```
| [Impacted Files](https://codecov.io/gh/apache/yunikorn-k8shim/pull/508?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/dispatcher/dispatcher.go](https://codecov.io/gh/apache/yunikorn-k8shim/pull/508/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2Rpc3BhdGNoZXIvZGlzcGF0Y2hlci5nbw==) | `74.82% <0.00%> (-1.40%)` | :arrow_down: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] zhuqi-lucas commented on a diff in pull request #508: [YUNIKORN-1499] Use fine-grained K8s access control in example deployments.
Posted by GitBox <gi...@apache.org>.
zhuqi-lucas commented on code in PR #508:
URL: https://github.com/apache/yunikorn-k8shim/pull/508#discussion_r1058835170
##########
deployments/scheduler/yunikorn-rbac.yaml:
##########
@@ -26,7 +66,49 @@ metadata:
subjects:
- kind: ServiceAccount
name: yunikorn-admin
+ namespace: yunikorn
+roleRef:
+ kind: ClusterRole
+ name: yunikorn-scheduler
+ apiGroup: rbac.authorization.k8s.io
+
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+ name: yunikorn-rbac-kube-scheduler
+subjects:
+ - kind: ServiceAccount
+ name: yunikorn-admin
+ namespace: yunikorn
+roleRef:
+ kind: ClusterRole
+ name: system:kube-scheduler
+ apiGroup: rbac.authorization.k8s.io
+
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+ name: yunikorn-rbac-volume-scheduler
+subjects:
+ - kind: ServiceAccount
+ name: yunikorn-admin
+ namespace: yunikorn
roleRef:
kind: ClusterRole
- name: cluster-admin
+ name: system:volume-scheduler
apiGroup: rbac.authorization.k8s.io
+
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+ name: yunikorn-rbac
+subjects:
+ - kind: ServiceAccount
+ name: yunikorn-admin
+roleRef:
+ kind: Role
+ name: yunikorn-scheduler
+ apiGroup: rbac.authorization.k8s.io
Review Comment:
Sure, add a newline now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org