You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "crain-cn (via GitHub)" <gi...@apache.org> on 2023/02/21 09:35:43 UTC

[GitHub] [incubator-uniffle] crain-cn opened a new pull request, #638: feat(operator): support specifying custom tolerations

crain-cn opened a new pull request, #638:
URL: https://github.com/apache/incubator-uniffle/pull/638

   ### What changes were proposed in this pull request?
   
   Support coordinator/shuffler server's  tolerations to the fields of RSS spec
   
   ### Why are the changes needed?
   1. Production environment deployment needs to use.
   2. In order to change parameters flexibly.
   
   ### Does this PR introduce _any_ user-facing change?
   For RSS cluster admin, they can set custom tolerations for shuffle servers and coordinators.
   
   ### How was this patch tested?
   Manually verified.
   ![image](https://user-images.githubusercontent.com/45311215/220305477-a9fd48aa-6857-4fbc-9df4-32bef2793746.png)
   
   


-- 
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] crain-cn commented on a diff in pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1113098486


##########
deploy/kubernetes/operator/api/uniffle/v1alpha1/remoteshuffleservice_types.go:
##########
@@ -218,6 +218,10 @@ type RSSPodSpec struct {
 	// Selector which must match a node's labels for the pod to be scheduled on that node.
 	// +optional
 	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+
+	// Indicates the tolerations the pods under this subset have.
+	// +optional
+	Tolerations []corev1.Toleration `json:"tolerations,omitempty"`

Review Comment:
   No need to add affinity, toleration configuration needs to be added in kubernetes 1.21.3 for our large-scale services, with many taints.



-- 
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 #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#issuecomment-1439511384

   > 
   > PRO asked me to add an affinity field, so I added another pr. #641
   
   I just took a look at #641, seems that it cover toleration and affinity both and includes some UT.  Do you prefer to close this in favor of #641? Actually you could just add more commits and finishes this PR? 
   
   Anyway is fine to me.


-- 
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] crain-cn commented on a diff in pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1113773316


##########
deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go:
##########
@@ -178,14 +178,9 @@ func GenerateDeploy(rss *unifflev1alpha1.RemoteShuffleService, index int) *appsv
 	podSpec := corev1.PodSpec{
 		HostNetwork:        *rss.Spec.Coordinator.HostNetwork,
 		ServiceAccountName: utils.GenerateCoordinatorName(rss),
-		Tolerations: []corev1.Toleration{
-			{
-				Effect: corev1.TaintEffectNoSchedule,
-				Key:    "node-role.kubernetes.io/master",
-			},
-		},
-		Volumes:      rss.Spec.Coordinator.Volumes,
-		NodeSelector: rss.Spec.Coordinator.NodeSelector,
+		Tolerations:        rss.Spec.Coordinator.Tolerations,
+		Volumes:            rss.Spec.Coordinator.Volumes,
+		NodeSelector:       rss.Spec.Coordinator.NodeSelector,
 	}
 	configurationVolume := corev1.Volume{

Review Comment:
   Thanks πŸ˜„ @advancedxy 



-- 
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] crain-cn commented on pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#issuecomment-1439522162

   > Then I close #641 
   
   


-- 
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 #638: feat(operator): support specifying custom tolerations

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1112898529


##########
deploy/kubernetes/operator/api/uniffle/v1alpha1/remoteshuffleservice_types.go:
##########
@@ -249,6 +253,15 @@ type MainContainer struct {
 	// VolumeMounts indicates describes mountings of volumes within shuffle servers' container.
 	// +optional
 	VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty"`
+
+	// NodeSelector is a selector which must be true for the pod to fit on a node.
+	// Selector which must match a node's labels for the pod to be scheduled on that node.
+	// +optional
+	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+
+	// Indicates the tolerations the pods under this subset have.
+	// +optional
+	Tolerations []corev1.Toleration `json:"tolerations,omitempty"`

Review Comment:
   It's not necessary to define `NodeSelector` and `Tolerations` in the MainContainer. It's already defined in the `RssPodSpec`



##########
deploy/kubernetes/operator/api/uniffle/v1alpha1/remoteshuffleservice_types.go:
##########
@@ -218,6 +218,10 @@ type RSSPodSpec struct {
 	// Selector which must match a node's labels for the pod to be scheduled on that node.
 	// +optional
 	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+
+	// Indicates the tolerations the pods under this subset have.
+	// +optional
+	Tolerations []corev1.Toleration `json:"tolerations,omitempty"`

Review Comment:
   cloud you also add affinity settings here?
   
   this pr could be an followup of  https://github.com/apache/incubator-uniffle/issues/545, which mentions to add toleration and affinity settings.



##########
deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go:
##########
@@ -178,12 +178,7 @@ func GenerateDeploy(rss *unifflev1alpha1.RemoteShuffleService, index int) *appsv
 	podSpec := corev1.PodSpec{
 		HostNetwork:        *rss.Spec.Coordinator.HostNetwork,
 		ServiceAccountName: utils.GenerateCoordinatorName(rss),
-		Tolerations: []corev1.Toleration{
-			{
-				Effect: corev1.TaintEffectNoSchedule,
-				Key:    "node-role.kubernetes.io/master",
-			},
-		},
+		Tolerations:  rss.Spec.Coordinator.Tolerations,

Review Comment:
   if toleration is not specified, please use the default toleration.



-- 
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] kaijchen commented on pull request #638: feat(operator): support specifying custom tolerations

Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#issuecomment-1438185401

   Thanks @crain-cn for the work. Please take a look at the test failures in CI.


-- 
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 #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1113756747


##########
deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go:
##########
@@ -178,12 +178,7 @@ func GenerateDeploy(rss *unifflev1alpha1.RemoteShuffleService, index int) *appsv
 	podSpec := corev1.PodSpec{
 		HostNetwork:        *rss.Spec.Coordinator.HostNetwork,
 		ServiceAccountName: utils.GenerateCoordinatorName(rss),
-		Tolerations: []corev1.Toleration{
-			{
-				Effect: corev1.TaintEffectNoSchedule,
-				Key:    "node-role.kubernetes.io/master",
-			},
-		},
+		Tolerations:  rss.Spec.Coordinator.Tolerations,

Review Comment:
   > Real business scenarios should not be deployed to the master rule node, especially for large single-node scales like ours, to consider the isolation of the business and ensure the stability of the business.
   
   what do you mean by large **singe-node** scales? 
   
   ----
   As for master rule toleration, it might be helpful to deploy rss to small clusters for test purpose. Otherwise, there wouldn't enough servers. So, it all depends your user cases.  Also cc @wangao1236, do you have more input on why master rule toleration is added previously.



-- 
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] crain-cn commented on pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#issuecomment-1439476728

   > Cloud you add some UTs for this new field?
   
   Already written.
   https://github.com/apache/incubator-uniffle/pull/641 [0cfc745](https://github.com/apache/incubator-uniffle/pull/641/commits/0cfc74516ac781caf4eb18422294cc29fa222090)
   


-- 
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] crain-cn commented on a diff in pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1113268599


##########
deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go:
##########
@@ -178,12 +178,7 @@ func GenerateDeploy(rss *unifflev1alpha1.RemoteShuffleService, index int) *appsv
 	podSpec := corev1.PodSpec{
 		HostNetwork:        *rss.Spec.Coordinator.HostNetwork,
 		ServiceAccountName: utils.GenerateCoordinatorName(rss),
-		Tolerations: []corev1.Toleration{
-			{
-				Effect: corev1.TaintEffectNoSchedule,
-				Key:    "node-role.kubernetes.io/master",
-			},
-		},
+		Tolerations:  rss.Spec.Coordinator.Tolerations,

Review Comment:
   Real business scenarios should not be deployed to the master rule node, especially for large single-node scales like ours, to consider the isolation of the business and ensure the stability of the business.



-- 
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 #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1113753527


##########
deploy/kubernetes/operator/api/uniffle/v1alpha1/remoteshuffleservice_types.go:
##########
@@ -218,6 +218,10 @@ type RSSPodSpec struct {
 	// Selector which must match a node's labels for the pod to be scheduled on that node.
 	// +optional
 	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+
+	// Indicates the tolerations the pods under this subset have.
+	// +optional
+	Tolerations []corev1.Toleration `json:"tolerations,omitempty"`

Review Comment:
   πŸ‘ , 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: 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] crain-cn commented on pull request #638: feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#issuecomment-1438195691

   > Thanks @crain-cn for the work. Please take a look at the test failures in CI.
   
   Thanks @kaijchen 


-- 
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] crain-cn commented on a diff in pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1113766324


##########
deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go:
##########
@@ -178,12 +178,7 @@ func GenerateDeploy(rss *unifflev1alpha1.RemoteShuffleService, index int) *appsv
 	podSpec := corev1.PodSpec{
 		HostNetwork:        *rss.Spec.Coordinator.HostNetwork,
 		ServiceAccountName: utils.GenerateCoordinatorName(rss),
-		Tolerations: []corev1.Toleration{
-			{
-				Effect: corev1.TaintEffectNoSchedule,
-				Key:    "node-role.kubernetes.io/master",
-			},
-		},
+		Tolerations:  rss.Spec.Coordinator.Tolerations,

Review Comment:
   > what do you mean by large **singe-node** scales?
   A cluster with many nodes. big cluster.
   



-- 
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] crain-cn commented on a diff in pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1113272205


##########
deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go:
##########
@@ -178,12 +178,7 @@ func GenerateDeploy(rss *unifflev1alpha1.RemoteShuffleService, index int) *appsv
 	podSpec := corev1.PodSpec{
 		HostNetwork:        *rss.Spec.Coordinator.HostNetwork,
 		ServiceAccountName: utils.GenerateCoordinatorName(rss),
-		Tolerations: []corev1.Toleration{
-			{
-				Effect: corev1.TaintEffectNoSchedule,
-				Key:    "node-role.kubernetes.io/master",
-			},
-		},
+		Tolerations:  rss.Spec.Coordinator.Tolerations,

Review Comment:
   If rss.Spec.Coordinator.Tolerations is not specified, can I modify the code to default to the original configuration?



-- 
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 #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#issuecomment-1439336135

   @crain-cn Could you add some uts for this pr?


-- 
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] crain-cn commented on a diff in pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1113886537


##########
deploy/kubernetes/operator/api/uniffle/v1alpha1/remoteshuffleservice_types.go:
##########
@@ -218,6 +218,10 @@ type RSSPodSpec struct {
 	// Selector which must match a node's labels for the pod to be scheduled on that node.
 	// +optional
 	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+
+	// Indicates the tolerations the pods under this subset have.
+	// +optional
+	Tolerations []corev1.Toleration `json:"tolerations,omitempty"`

Review Comment:
   > > > @crain-cn Could you add some uts for this pr?
   > > > Already written.
   > > > #641 [0cfc745](https://github.com/apache/incubator-uniffle/pull/641/commits/0cfc74516ac781caf4eb18422294cc29fa222090)
   > 
   > We would better add this pr's ut to this pr. If we need integration, we could add it in another single pr.
   
   PRO asked me to add the affinity field, so I added another pr.  #641 
   
   



-- 
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 #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#issuecomment-1439411423

   > @advancedxy Affinity fields , can I submit to the next pr?
   
   of course.


-- 
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] crain-cn closed pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn closed pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations
URL: https://github.com/apache/incubator-uniffle/pull/638


-- 
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] zuston commented on pull request #638: feat(operator): support specifying custom tolerations

Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#issuecomment-1438164518

   Could you help review this? @wangao1236 @advancedxy 


-- 
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 #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#issuecomment-1438387197

   # [Codecov](https://codecov.io/gh/apache/incubator-uniffle/pull/638?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 [#638](https://codecov.io/gh/apache/incubator-uniffle/pull/638?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d68a40f) into [master](https://codecov.io/gh/apache/incubator-uniffle/commit/c45c187b6905b37d93abfc8e0ab2b5829ab18096?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c45c187) will **increase** coverage by `2.30%`.
   > The diff coverage is `n/a`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #638      +/-   ##
   ============================================
   + Coverage     60.89%   63.19%   +2.30%     
   - Complexity     1798     1799       +1     
   ============================================
     Files           214      200      -14     
     Lines         12381    10410    -1971     
     Branches       1042     1042              
   ============================================
   - Hits           7539     6579     -960     
   + Misses         4438     3488     -950     
   + Partials        404      343      -61     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-uniffle/pull/638?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/638?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/638?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=) | | |
   | [...bernetes/operator/pkg/controller/controller/rss.go](https://codecov.io/gh/apache/incubator-uniffle/pull/638?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=) | | |
   | [...y/kubernetes/operator/pkg/webhook/inspector/rss.go](https://codecov.io/gh/apache/incubator-uniffle/pull/638?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3dlYmhvb2svaW5zcGVjdG9yL3Jzcy5nbw==) | | |
   | [...y/kubernetes/operator/pkg/webhook/inspector/pod.go](https://codecov.io/gh/apache/incubator-uniffle/pull/638?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3dlYmhvb2svaW5zcGVjdG9yL3BvZC5nbw==) | | |
   | [...eploy/kubernetes/operator/pkg/utils/coordinator.go](https://codecov.io/gh/apache/incubator-uniffle/pull/638?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3V0aWxzL2Nvb3JkaW5hdG9yLmdv) | | |
   | [...oy/kubernetes/operator/pkg/controller/util/util.go](https://codecov.io/gh/apache/incubator-uniffle/pull/638?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) | | |
   | [...oy/kubernetes/operator/pkg/utils/shufflerserver.go](https://codecov.io/gh/apache/incubator-uniffle/pull/638?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3V0aWxzL3NodWZmbGVyc2VydmVyLmdv) | | |
   | [deploy/kubernetes/operator/pkg/utils/rss.go](https://codecov.io/gh/apache/incubator-uniffle/pull/638?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/638?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) | | |
   | ... and [5 more](https://codecov.io/gh/apache/incubator-uniffle/pull/638?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] advancedxy commented on a diff in pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1113757686


##########
deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go:
##########
@@ -178,14 +178,9 @@ func GenerateDeploy(rss *unifflev1alpha1.RemoteShuffleService, index int) *appsv
 	podSpec := corev1.PodSpec{
 		HostNetwork:        *rss.Spec.Coordinator.HostNetwork,
 		ServiceAccountName: utils.GenerateCoordinatorName(rss),
-		Tolerations: []corev1.Toleration{
-			{
-				Effect: corev1.TaintEffectNoSchedule,
-				Key:    "node-role.kubernetes.io/master",
-			},
-		},
-		Volumes:      rss.Spec.Coordinator.Volumes,
-		NodeSelector: rss.Spec.Coordinator.NodeSelector,
+		Tolerations:        rss.Spec.Coordinator.Tolerations,
+		Volumes:            rss.Spec.Coordinator.Volumes,
+		NodeSelector:       rss.Spec.Coordinator.NodeSelector,
 	}
 	configurationVolume := corev1.Volume{

Review Comment:
   ```suggestion
           if len(podSpec.Tolerations) == 0 {
              podSpec.Tolerations = []corev1.Toleration{...} # the default master rule toleration
           }
   	configurationVolume := corev1.Volume{
   	
   ```



-- 
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] crain-cn commented on pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#issuecomment-1439412127

   > > @advancedxy Affinity fields , can I submit to the next pr?
   > 
   > of course.
   
   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: 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] crain-cn commented on pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#issuecomment-1439367755

   > @crain-cn Could you add some uts for this pr?
   Yeah.
   


-- 
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] crain-cn commented on pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#issuecomment-1439476222

   > @crain-cn Could you add some uts for this pr?
   Already written.
   #641 [0cfc745](https://github.com/apache/incubator-uniffle/pull/641/commits/0cfc74516ac781caf4eb18422294cc29fa222090)


-- 
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] crain-cn commented on a diff in pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1113774898


##########
deploy/kubernetes/operator/api/uniffle/v1alpha1/remoteshuffleservice_types.go:
##########
@@ -218,6 +218,10 @@ type RSSPodSpec struct {
 	// Selector which must match a node's labels for the pod to be scheduled on that node.
 	// +optional
 	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+
+	// Indicates the tolerations the pods under this subset have.
+	// +optional
+	Tolerations []corev1.Toleration `json:"tolerations,omitempty"`

Review Comment:
   > πŸ‘ , thanks.
   Affinities, can I submit to the next pr?
   
   
   



-- 
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] crain-cn commented on pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#issuecomment-1439449805

   > @crain-cn Could you add some uts for this pr?
   
   Can I make it up later?
   
   


-- 
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] crain-cn commented on pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#issuecomment-1439503591

   > > > @crain-cn Could you add some uts for this pr?
   > > > Already written.
   > > > #641 [0cfc745](https://github.com/apache/incubator-uniffle/pull/641/commits/0cfc74516ac781caf4eb18422294cc29fa222090)
   > 
   > We would better add this pr's ut to this pr. If we need integration, we could add it in another single pr.
   
   PRO asked me to add an affinity field, so I added another pr. #641 
   
   
   


-- 
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 #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1113214192


##########
deploy/kubernetes/operator/api/uniffle/v1alpha1/remoteshuffleservice_types.go:
##########
@@ -218,6 +218,10 @@ type RSSPodSpec struct {
 	// Selector which must match a node's labels for the pod to be scheduled on that node.
 	// +optional
 	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+
+	// Indicates the tolerations the pods under this subset have.
+	// +optional
+	Tolerations []corev1.Toleration `json:"tolerations,omitempty"`

Review Comment:
   Yeah, I know you don't need to add affinity to support your case. But affinity and tolerations are commonly used together for flexible scheduling semantics.  We definitely want to support both tolerations and affinities. So I'm kindly asking whether you could also add the affinity fields here. 
   
   If it's too much work for this pr, we can defer the affinity support later.



##########
deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go:
##########
@@ -178,12 +178,7 @@ func GenerateDeploy(rss *unifflev1alpha1.RemoteShuffleService, index int) *appsv
 	podSpec := corev1.PodSpec{
 		HostNetwork:        *rss.Spec.Coordinator.HostNetwork,
 		ServiceAccountName: utils.GenerateCoordinatorName(rss),
-		Tolerations: []corev1.Toleration{
-			{
-				Effect: corev1.TaintEffectNoSchedule,
-				Key:    "node-role.kubernetes.io/master",
-			},
-		},
+		Tolerations:  rss.Spec.Coordinator.Tolerations,

Review Comment:
   > If using the default configuration, it is definitely not supported. 
   
   cloud you elaborate a bit more on why it is not supported?
   
   > if toleration is not specified, please use the default toleration.
   
   What I mean is that, if the `rss.Spec.Coordinator.Tolerations` is not specified or empty, we should use the previous toleration, which means rss server could be deployed to master rule nodes.



-- 
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] crain-cn commented on a diff in pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1113773316


##########
deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go:
##########
@@ -178,14 +178,9 @@ func GenerateDeploy(rss *unifflev1alpha1.RemoteShuffleService, index int) *appsv
 	podSpec := corev1.PodSpec{
 		HostNetwork:        *rss.Spec.Coordinator.HostNetwork,
 		ServiceAccountName: utils.GenerateCoordinatorName(rss),
-		Tolerations: []corev1.Toleration{
-			{
-				Effect: corev1.TaintEffectNoSchedule,
-				Key:    "node-role.kubernetes.io/master",
-			},
-		},
-		Volumes:      rss.Spec.Coordinator.Volumes,
-		NodeSelector: rss.Spec.Coordinator.NodeSelector,
+		Tolerations:        rss.Spec.Coordinator.Tolerations,
+		Volumes:            rss.Spec.Coordinator.Volumes,
+		NodeSelector:       rss.Spec.Coordinator.NodeSelector,
 	}
 	configurationVolume := corev1.Volume{

Review Comment:
   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: 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] crain-cn commented on a diff in pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1113774898


##########
deploy/kubernetes/operator/api/uniffle/v1alpha1/remoteshuffleservice_types.go:
##########
@@ -218,6 +218,10 @@ type RSSPodSpec struct {
 	// Selector which must match a node's labels for the pod to be scheduled on that node.
 	// +optional
 	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+
+	// Indicates the tolerations the pods under this subset have.
+	// +optional
+	Tolerations []corev1.Toleration `json:"tolerations,omitempty"`

Review Comment:
   > πŸ‘ , thanks.
   Affinity fields , can I submit to the next pr?
   
   
   



-- 
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] crain-cn commented on pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#issuecomment-1439402143

   @advancedxy Affinity fields , can I submit to the next pr?


-- 
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] crain-cn commented on a diff in pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1113101964


##########
deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go:
##########
@@ -178,12 +178,7 @@ func GenerateDeploy(rss *unifflev1alpha1.RemoteShuffleService, index int) *appsv
 	podSpec := corev1.PodSpec{
 		HostNetwork:        *rss.Spec.Coordinator.HostNetwork,
 		ServiceAccountName: utils.GenerateCoordinatorName(rss),
-		Tolerations: []corev1.Toleration{
-			{
-				Effect: corev1.TaintEffectNoSchedule,
-				Key:    "node-role.kubernetes.io/master",
-			},
-		},
+		Tolerations:  rss.Spec.Coordinator.Tolerations,

Review Comment:
   If using the default configuration, it is definitely not supported. The operator must be modified. If not supported, we can only do secondary development here. We really hope to cooperate with you here. Our single node scale is above 4000 nodes, and we also hope that the community can provide help. Thank you!



-- 
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] crain-cn commented on a diff in pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1113095335


##########
deploy/kubernetes/operator/api/uniffle/v1alpha1/remoteshuffleservice_types.go:
##########
@@ -249,6 +253,15 @@ type MainContainer struct {
 	// VolumeMounts indicates describes mountings of volumes within shuffle servers' container.
 	// +optional
 	VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty"`
+
+	// NodeSelector is a selector which must be true for the pod to fit on a node.
+	// Selector which must match a node's labels for the pod to be scheduled on that node.
+	// +optional
+	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+
+	// Indicates the tolerations the pods under this subset have.
+	// +optional
+	Tolerations []corev1.Toleration `json:"tolerations,omitempty"`

Review Comment:
   The fields in MainContainer have been removed.



-- 
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] crain-cn commented on a diff in pull request #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "crain-cn (via GitHub)" <gi...@apache.org>.
crain-cn commented on code in PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#discussion_r1113280909


##########
deploy/kubernetes/operator/api/uniffle/v1alpha1/remoteshuffleservice_types.go:
##########
@@ -218,6 +218,10 @@ type RSSPodSpec struct {
 	// Selector which must match a node's labels for the pod to be scheduled on that node.
 	// +optional
 	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+
+	// Indicates the tolerations the pods under this subset have.
+	// +optional
+	Tolerations []corev1.Toleration `json:"tolerations,omitempty"`

Review Comment:
   I can support this together and submit a pull request.



-- 
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 #638: [#545][FOLLOWUP]feat(operator): support specifying custom tolerations

Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on PR #638:
URL: https://github.com/apache/incubator-uniffle/pull/638#issuecomment-1439495522

   > > @crain-cn Could you add some uts for this pr?
   > > Already written.
   > > #641 [0cfc745](https://github.com/apache/incubator-uniffle/pull/641/commits/0cfc74516ac781caf4eb18422294cc29fa222090)
   
   We would better add this pr's ut to this pr. If we need integration, we could add it in another single pr.


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