You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by GitBox <gi...@apache.org> on 2022/12/02 08:56:40 UTC
[GitHub] [incubator-celeborn] pan3793 opened a new pull request, #895: Celeborn should run as celeborn instead of root
pan3793 opened a new pull request, #895:
URL: https://github.com/apache/incubator-celeborn/pull/895
# [BUG]/[FEATURE] title
### What changes were proposed in this pull request?
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### What are the items that need reviewer attention?
### Related issues.
### Related pull requests.
### How was this patch tested?
/cc @related-reviewer
/assign @main-reviewer
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] FMX commented on a diff in pull request #895: [CELEBORN-151][K8S] Celeborn should run as celeborn instead of root in container
Posted by GitBox <gi...@apache.org>.
FMX commented on code in PR #895:
URL: https://github.com/apache/incubator-celeborn/pull/895#discussion_r1052996317
##########
docker/helm/templates/master-statefulset.yaml:
##########
@@ -58,6 +58,8 @@ spec:
tolerations:
{{- toYaml . | nindent 8 }}
{{- end }}
+ securityContext:
+ fsGroup: {{ .Values.gid }}
Review Comment:
I think that's ok.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] pan3793 commented on a diff in pull request #895: [CELEBORN-151][K8S] Celeborn should run as celeborn instead of root in container
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #895:
URL: https://github.com/apache/incubator-celeborn/pull/895#discussion_r1052974389
##########
docker/helm/templates/master-statefulset.yaml:
##########
@@ -58,6 +58,8 @@ spec:
tolerations:
{{- toYaml . | nindent 8 }}
{{- end }}
+ securityContext:
+ fsGroup: {{ .Values.gid }}
Review Comment:
> `fsGroupPolicystring`
> Defines if the underlying volume supports changing ownership and permission of the volume before being mounted. Refer to the specific FSGroupPolicy values for additional details. This field is alpha-level, and is only honored by servers that enable the CSIVolumeFSGroupPolicy feature gate.
https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.20/
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] pan3793 commented on a diff in pull request #895: [CELEBORN-151][K8S] Celeborn should run as celeborn instead of root in container
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #895:
URL: https://github.com/apache/incubator-celeborn/pull/895#discussion_r1052957364
##########
docker/helm/values.yaml:
##########
@@ -36,35 +36,36 @@ workerReplicas: 5
# celeborn release version
celebornVersion: 0.1.1
+gid: 10006
+
# celeborn configurations
celeborn:
- # please update celeborn.worker.storage.dirs to disk mount path on k8s node
+ celeborn.ha.master.ratis.raft.server.storage.dir: /mnt/rss_ratis/
celeborn.worker.storage.dirs: /mnt/disk1,/mnt/disk2,/mnt/disk3,/mnt/disk4
celeborn.push.replicate.enabled: true
+ celeborn.metrics.enabled: true
celeborn.master.metrics.prometheus.port: 9098
+ celeborn.worker.metrics.prometheus.port: 9096
celeborn.worker.monitor.disk.enabled: false
- rss.rpc.io.serverThreads: 64
- rss.worker.fetch.chunk.size: 8m
- rss.rpc.io.numConnectionsPerPeer: 2
+ celeborn.worker.fetch.chunk.size: 8m
+ celeborn.rpc.io.serverThreads: 64
+ celeborn.rpc.io.numConnectionsPerPeer: 2
+ celeborn.rpc.io.clientThreads: 64
+ celeborn.rpc.dispatcher.numThreads: 4
celeborn.worker.flush.buffer.size: 256K
- celeborn.metrics.enabled: true
- rss.push.io.threads: 32
celeborn.worker.fetch.io.threads: 32
+ celeborn.push.io.threads: 32
Review Comment:
it's there.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] github-actions[bot] commented on pull request #895: Celeborn should run as celeborn instead of root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #895:
URL: https://github.com/apache/incubator-celeborn/pull/895#issuecomment-1321693176
This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days.
--
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: dev-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] codecov[bot] commented on pull request #895: Celeborn should run as celeborn instead of root
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #895:
URL: https://github.com/apache/incubator-celeborn/pull/895#issuecomment-1354316465
# [Codecov](https://codecov.io/gh/apache/incubator-celeborn/pull/895?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 [#895](https://codecov.io/gh/apache/incubator-celeborn/pull/895?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1137d3c) into [main](https://codecov.io/gh/apache/incubator-celeborn/commit/dc66369973c67d0c06250084aff8efb9aad96a4e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (dc66369) will **not change** coverage.
> The diff coverage is `n/a`.
> :exclamation: Current head 1137d3c differs from pull request most recent head 84a2e1b. Consider uploading reports for the commit 84a2e1b to get more accurate results
```diff
@@ Coverage Diff @@
## main #895 +/- ##
=========================================
Coverage 25.03% 25.03%
Complexity 770 770
=========================================
Files 215 215
Lines 18195 18195
Branches 2035 2035
=========================================
Hits 4553 4553
Misses 13331 13331
Partials 311 311
```
: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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] pan3793 commented on pull request #895: [CELEBORN-151][K8S] Celeborn should run as celeborn instead of root in container
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #895:
URL: https://github.com/apache/incubator-celeborn/pull/895#issuecomment-1358889261
cc @FMX @zwangsheng
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] FMX merged pull request #895: [CELEBORN-151][K8S] Celeborn should run as celeborn instead of root in container
Posted by GitBox <gi...@apache.org>.
FMX merged PR #895:
URL: https://github.com/apache/incubator-celeborn/pull/895
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] github-actions[bot] commented on pull request #895: Celeborn should run as celeborn instead of root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #895:
URL: https://github.com/apache/incubator-celeborn/pull/895#issuecomment-1334906732
This issue was closed because it has been staled for 10 days with no activity.
--
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: dev-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] github-actions[bot] closed pull request #895: Celeborn should run as celeborn instead of root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #895: Celeborn should run as celeborn instead of root
URL: https://github.com/apache/incubator-celeborn/pull/895
--
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: dev-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] FMX commented on a diff in pull request #895: [CELEBORN-151][K8S] Celeborn should run as celeborn instead of root in container
Posted by GitBox <gi...@apache.org>.
FMX commented on code in PR #895:
URL: https://github.com/apache/incubator-celeborn/pull/895#discussion_r1052952228
##########
docker/helm/values.yaml:
##########
@@ -36,35 +36,36 @@ workerReplicas: 5
# celeborn release version
celebornVersion: 0.1.1
+gid: 10006
+
# celeborn configurations
celeborn:
- # please update celeborn.worker.storage.dirs to disk mount path on k8s node
+ celeborn.ha.master.ratis.raft.server.storage.dir: /mnt/rss_ratis/
celeborn.worker.storage.dirs: /mnt/disk1,/mnt/disk2,/mnt/disk3,/mnt/disk4
celeborn.push.replicate.enabled: true
+ celeborn.metrics.enabled: true
celeborn.master.metrics.prometheus.port: 9098
+ celeborn.worker.metrics.prometheus.port: 9096
celeborn.worker.monitor.disk.enabled: false
- rss.rpc.io.serverThreads: 64
- rss.worker.fetch.chunk.size: 8m
- rss.rpc.io.numConnectionsPerPeer: 2
+ celeborn.worker.fetch.chunk.size: 8m
+ celeborn.rpc.io.serverThreads: 64
+ celeborn.rpc.io.numConnectionsPerPeer: 2
+ celeborn.rpc.io.clientThreads: 64
+ celeborn.rpc.dispatcher.numThreads: 4
celeborn.worker.flush.buffer.size: 256K
- celeborn.metrics.enabled: true
- rss.push.io.threads: 32
celeborn.worker.fetch.io.threads: 32
+ celeborn.push.io.threads: 32
Review Comment:
we should add "celeborn.worker.fetch.io.threads: 32" here because the PVC will be regarded as one disk.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org