You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "advancedxy (via GitHub)" <gi...@apache.org> on 2023/02/20 14:04:41 UTC
[GitHub] [incubator-uniffle] advancedxy opened a new pull request, #634: [#632]fix: respect volumes in rss spec
advancedxy opened a new pull request, #634:
URL: https://github.com/apache/incubator-uniffle/pull/634
### What changes were proposed in this pull request?
respect volumes for rss coordinator and shuffle server
### Why are the changes needed?
This is a bug fix
Fix: #632
### Does this PR introduce _any_ user-facing change?
Admin of rss cluster could specify the volumes which could be used to configure
hadoop conf for example
### How was this patch tested?
Added UTs.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #634: [#632]fix: respect volumes in rss spec
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #634:
URL: https://github.com/apache/incubator-uniffle/pull/634#issuecomment-1437738636
Hi, @wangao1236 @jerqi please take a look.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #634: [#632]fix: respect volumes in rss spec
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #634:
URL: https://github.com/apache/incubator-uniffle/pull/634#issuecomment-1437762277
> Is it duplicated with this pr? #466
No. They are two different issues. This is a bug fix.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on pull request #634: [#632]fix: respect volumes in rss spec
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on PR #634:
URL: https://github.com/apache/incubator-uniffle/pull/634#issuecomment-1437751292
Is it duplicated with this pr? https://github.com/apache/incubator-uniffle/pull/466
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on pull request #634: [#632]fix: respect volumes in rss spec
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on PR #634:
URL: https://github.com/apache/incubator-uniffle/pull/634#issuecomment-1438014806
merged master & branch-0.7
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #634: [#632]fix: respect volumes in rss spec
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #634:
URL: https://github.com/apache/incubator-uniffle/pull/634#issuecomment-1437075212
This is bug fix, and it should also be included in branch-0.7. 😢 😠cc @zuston
I will cherry-pick this pr once 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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi merged pull request #634: [#632]fix: respect volumes in rss spec
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi merged PR #634:
URL: https://github.com/apache/incubator-uniffle/pull/634
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #634: [#632]fix: respect volumes in rss spec
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #634:
URL: https://github.com/apache/incubator-uniffle/pull/634#discussion_r1112605279
##########
deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver_test.go:
##########
@@ -315,6 +334,18 @@ func TestGenerateSts(t *testing.T) {
return
},
},
+ {
+ name: "test custom volumes",
+ rss: withCustomVolumes(testVolumes),
+ IsValidSts: func(sts *appsv1.StatefulSet, rss *uniffleapi.RemoteShuffleService) (valid bool, err error) {
+ for _, volume := range sts.Spec.Template.Spec.Volumes {
+ if volume.Name == testVolumeName {
+ return true, nil
Review Comment:
addressed. Please take a look again
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] codecov-commenter commented on pull request #634: [#632]fix: respect volumes in rss spec
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #634:
URL: https://github.com/apache/incubator-uniffle/pull/634#issuecomment-1437088567
# [Codecov](https://codecov.io/gh/apache/incubator-uniffle/pull/634?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 [#634](https://codecov.io/gh/apache/incubator-uniffle/pull/634?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4bcfe6f) into [master](https://codecov.io/gh/apache/incubator-uniffle/commit/b31a5c9fff01948bce159f9c95cb58cc95e2f9cb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b31a5c9) will **increase** coverage by `2.29%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #634 +/- ##
============================================
+ Coverage 60.90% 63.19% +2.29%
Complexity 1799 1799
============================================
Files 214 200 -14
Lines 12381 10410 -1971
Branches 1042 1042
============================================
- Hits 7541 6579 -962
+ Misses 4437 3488 -949
+ Partials 403 343 -60
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-uniffle/pull/634?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...tor/pkg/controller/sync/coordinator/coordinator.go](https://codecov.io/gh/apache/incubator-uniffle/pull/634?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL2NvbnRyb2xsZXIvc3luYy9jb29yZGluYXRvci9jb29yZGluYXRvci5nbw==) | | |
| [...pkg/controller/sync/shuffleserver/shuffleserver.go](https://codecov.io/gh/apache/incubator-uniffle/pull/634?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL2NvbnRyb2xsZXIvc3luYy9zaHVmZmxlc2VydmVyL3NodWZmbGVzZXJ2ZXIuZ28=) | | |
| [...oy/kubernetes/operator/pkg/controller/util/util.go](https://codecov.io/gh/apache/incubator-uniffle/pull/634?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL2NvbnRyb2xsZXIvdXRpbC91dGlsLmdv) | | |
| [deploy/kubernetes/operator/pkg/webhook/manager.go](https://codecov.io/gh/apache/incubator-uniffle/pull/634?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3dlYmhvb2svbWFuYWdlci5nbw==) | | |
| [deploy/kubernetes/operator/pkg/utils/config.go](https://codecov.io/gh/apache/incubator-uniffle/pull/634?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3V0aWxzL2NvbmZpZy5nbw==) | | |
| [deploy/kubernetes/operator/pkg/utils/rss.go](https://codecov.io/gh/apache/incubator-uniffle/pull/634?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3V0aWxzL3Jzcy5nbw==) | | |
| [deploy/kubernetes/operator/pkg/utils/certs.go](https://codecov.io/gh/apache/incubator-uniffle/pull/634?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3V0aWxzL2NlcnRzLmdv) | | |
| [...rnetes/operator/pkg/webhook/inspector/inspector.go](https://codecov.io/gh/apache/incubator-uniffle/pull/634?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3dlYmhvb2svaW5zcGVjdG9yL2luc3BlY3Rvci5nbw==) | | |
| [...bernetes/operator/pkg/controller/controller/rss.go](https://codecov.io/gh/apache/incubator-uniffle/pull/634?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL2NvbnRyb2xsZXIvY29udHJvbGxlci9yc3MuZ28=) | | |
| [deploy/kubernetes/operator/pkg/utils/util.go](https://codecov.io/gh/apache/incubator-uniffle/pull/634?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3V0aWxzL3V0aWwuZ28=) | | |
| ... and [4 more](https://codecov.io/gh/apache/incubator-uniffle/pull/634?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
: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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] wangao1236 commented on a diff in pull request #634: [#632]fix: respect volumes in rss spec
Posted by "wangao1236 (via GitHub)" <gi...@apache.org>.
wangao1236 commented on code in PR #634:
URL: https://github.com/apache/incubator-uniffle/pull/634#discussion_r1112571294
##########
deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver_test.go:
##########
@@ -315,6 +334,18 @@ func TestGenerateSts(t *testing.T) {
return
},
},
+ {
+ name: "test custom volumes",
+ rss: withCustomVolumes(testVolumes),
+ IsValidSts: func(sts *appsv1.StatefulSet, rss *uniffleapi.RemoteShuffleService) (valid bool, err error) {
+ for _, volume := range sts.Spec.Template.Spec.Volumes {
+ if volume.Name == testVolumeName {
+ return true, nil
Review Comment:
Maybe we can compare the contents of your custom volume by deepcopy method?
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org