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/01/07 03:39:42 UTC

[GitHub] [incubator-yunikorn-k8shim] anuraagnalluri opened a new pull request #348: [YUNIKORN-995] Use v1 for ClusterRoleBinding rbac.authorization.k8s.io instead of v1beta1

anuraagnalluri opened a new pull request #348:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/348


   ### What is this PR for?
   Helm installation for Yunikorn using Kubernetes version 1.22 currently fails, see details [here](https://issues.apache.org/jira/browse/YUNIKORN-995) and tickets referenced in the comments. This PR was originally intended to upgrade the `apiVersion` of `ClusterRoleBinding` from `v1beta -> v1`, but fixing that issue alone did not enable a successful helm installation of Yunikorn using K8s 1.22. As a result, this PR has come to encompass other changes required to do so.
   
   ### What type of PR is it?
   * [x] - Bug Fix
   * [ ] - Improvement
   * [ ] - Feature
   * [ ] - Documentation
   * [ ] - Hot Fix
   * [ ] - Refactoring
   
   ### Todos
   * [x] - Fix cert issue with admission controller -- currently blocking successful deployment of admission controller
   * [x] - Verify changes don't break ability to helm install Yunikorn with K8s 1.19+
   
   ### What is the Jira issue?
   https://issues.apache.org/jira/browse/YUNIKORN-995
   
   ### How should this be tested?
   `make image` locally and use the existing `0.12.1` helm chart to ensure scheduler/admission controller can be brought up successfully with all K8s versions 1.19+
   
   ### 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] [incubator-yunikorn-k8shim] anuraagnalluri commented on pull request #348: [YUNIKORN-995] Use v1 for ClusterRoleBinding rbac.authorization.k8s.io instead of v1beta1

Posted by GitBox <gi...@apache.org>.
anuraagnalluri commented on pull request #348:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/348#issuecomment-1010932878


   No longer relevant after decoupling of scheduler/admission-controller effort. 


-- 
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] [incubator-yunikorn-k8shim] anuraagnalluri commented on pull request #348: [YUNIKORN-995] Use v1 for ClusterRoleBinding rbac.authorization.k8s.io instead of v1beta1

Posted by GitBox <gi...@apache.org>.
anuraagnalluri commented on pull request #348:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/348#issuecomment-1007119506


   @yangwwei Sorry if these changes are too broad for the context of [YUNIKORN-995](https://issues.apache.org/jira/browse/YUNIKORN-995). I can make an umbrella ticket for enabling helm installation on K8s 1.22 and have this PR reference that instead if that's preferable.
   
   If we want to target this for 1.0 release, does it make sense for me to PR this to `master` now? From reading the [release procedure](https://github.com/apache/incubator-yunikorn-release/blob/master/docs/release-procedure.md), I was under the impression that this makes sense but please correct me if I'm wrong.


-- 
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] [incubator-yunikorn-k8shim] anuraagnalluri edited a comment on pull request #348: [YUNIKORN-995] Use v1 for ClusterRoleBinding rbac.authorization.k8s.io instead of v1beta1

Posted by GitBox <gi...@apache.org>.
anuraagnalluri edited a comment on pull request #348:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/348#issuecomment-1007119506


   @yangwwei Sorry if these changes are too broad for the context of [YUNIKORN-995](https://issues.apache.org/jira/browse/YUNIKORN-995). I can make an umbrella ticket for enabling helm installation on K8s 1.22 and have this PR reference that instead if that's preferable.
   
   Edit: Became aware of [efforts to split scheduler/admission-controller deployment ](https://github.com/apache/incubator-yunikorn-k8shim/pull/346) which will fix the cert issues I'm facing with 1.22. Will rebase after that's merged.


-- 
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] [incubator-yunikorn-k8shim] codecov[bot] commented on pull request #348: [YUNIKORN-995] Use v1 for ClusterRoleBinding rbac.authorization.k8s.io instead of v1beta1

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #348:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/348#issuecomment-1007122093


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348?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 [#348](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (196d34b) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/c47ed51f075c5af5910f71da40e7e68699a9abae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c47ed51) will **increase** coverage by `2.21%`.
   > The diff coverage is `63.35%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #348      +/-   ##
   ==========================================
   + Coverage   59.75%   61.96%   +2.21%     
   ==========================================
     Files          35       39       +4     
     Lines        3133     5132    +1999     
   ==========================================
   + Hits         1872     3180    +1308     
   - Misses       1180     1846     +666     
   - Partials       81      106      +25     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348?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/cache/amprotocol\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348/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-cGtnL2NhY2hlL2FtcHJvdG9jb2xfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
   | [pkg/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348/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-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `40.67% <0.00%> (-5.67%)` | :arrow_down: |
   | [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348/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-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXJfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
   | [pkg/common/events/states.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348/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-cGtnL2NvbW1vbi9ldmVudHMvc3RhdGVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/shim/main.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348/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-cGtnL3NoaW0vbWFpbi5nbw==) | `0.00% <0.00%> (ø)` | |
   | [...missioncontrollers/webhook/admission\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348/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-cGtnL3BsdWdpbi9hZG1pc3Npb25jb250cm9sbGVycy93ZWJob29rL2FkbWlzc2lvbl9jb250cm9sbGVyLmdv) | `34.43% <5.71%> (+1.69%)` | :arrow_up: |
   | [pkg/appmgmt/appmgmt\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348/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-cGtnL2FwcG1nbXQvYXBwbWdtdF9yZWNvdmVyeS5nbw==) | `67.92% <16.66%> (-7.76%)` | :arrow_down: |
   | [pkg/cache/context.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348/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-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `37.82% <19.29%> (-3.26%)` | :arrow_down: |
   | [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348/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-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXIuZ28=) | `40.00% <28.57%> (+10.00%)` | :arrow_up: |
   | [pkg/cache/nodes.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348/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-cGtnL2NhY2hlL25vZGVzLmdv) | `78.28% <46.15%> (-1.52%)` | :arrow_down: |
   | ... and [55 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [1e45e53...196d34b](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/348?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-k8shim] anuraagnalluri closed pull request #348: [YUNIKORN-995] Use v1 for ClusterRoleBinding rbac.authorization.k8s.io instead of v1beta1

Posted by GitBox <gi...@apache.org>.
anuraagnalluri closed pull request #348:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/348


   


-- 
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] [incubator-yunikorn-k8shim] anuraagnalluri edited a comment on pull request #348: [YUNIKORN-995] Use v1 for ClusterRoleBinding rbac.authorization.k8s.io instead of v1beta1

Posted by GitBox <gi...@apache.org>.
anuraagnalluri edited a comment on pull request #348:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/348#issuecomment-1007119506


   @yangwwei Sorry if these changes are too broad for the context of [YUNIKORN-995](https://issues.apache.org/jira/browse/YUNIKORN-995). I can make an umbrella ticket for enabling helm installation on K8s 1.22 and have this PR reference that instead if that's preferable.
   
   Edit: Became aware of [efforts to split scheduler/admission-controller deployment ](https://github.com/apache/incubator-yunikorn-k8shim/pull/346) which may fix the cert issues I'm facing with 1.22. Will rebase after that's merged.


-- 
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