You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by GitBox <gi...@apache.org> on 2023/01/11 03:52:49 UTC

[GitHub] [incubator-uniffle] wangao1236 commented on a diff in pull request #466: [ISSUE-288] Support mounting PVC for shuffleserver

wangao1236 commented on code in PR #466:
URL: https://github.com/apache/incubator-uniffle/pull/466#discussion_r1066543820


##########
deploy/kubernetes/operator/api/uniffle/v1alpha1/remoteshuffleservice_types.go:
##########
@@ -136,6 +136,29 @@ type ShuffleServerConfig struct {
 
 	// UpgradeStrategy defines upgrade strategy of shuffle servers.
 	UpgradeStrategy *ShuffleServerUpgradeStrategy `json:"upgradeStrategy"`
+
+	// volumeClaimTemplates is a list of claims that pods are allowed to reference.
+	// The StatefulSet controller is responsible for mapping network identities to
+	// claims in a way that maintains the identity of a pod. Every claim in
+	// this list must have at least one matching (by name) volumeMount in one
+	// container in the template. A claim in this list takes precedence over
+	// any volumes in the template, with the same name.
+	// +optional
+	VolumeClaimTemplates []ShuffleServerPersistentVolumeClaimTemplate `json:"volumeClaimTemplates,omitempty" protobuf:"bytes,4,rep,name=volumeClaimTemplates"`

Review Comment:
   Why don't you use "[]v1.PersistentVolumeClaim" as StatefulSet? 
   Also, are there any scenarios where protobuf is currently used?



##########
deploy/kubernetes/operator/api/uniffle/v1alpha1/remoteshuffleservice_types.go:
##########
@@ -136,6 +136,29 @@ type ShuffleServerConfig struct {
 
 	// UpgradeStrategy defines upgrade strategy of shuffle servers.
 	UpgradeStrategy *ShuffleServerUpgradeStrategy `json:"upgradeStrategy"`
+
+	// volumeClaimTemplates is a list of claims that pods are allowed to reference.
+	// The StatefulSet controller is responsible for mapping network identities to
+	// claims in a way that maintains the identity of a pod. Every claim in
+	// this list must have at least one matching (by name) volumeMount in one
+	// container in the template. A claim in this list takes precedence over
+	// any volumes in the template, with the same name.
+	// +optional
+	VolumeClaimTemplates []ShuffleServerPersistentVolumeClaimTemplate `json:"volumeClaimTemplates,omitempty" protobuf:"bytes,4,rep,name=volumeClaimTemplates"`

Review Comment:
   Why don't you use "[]v1.PersistentVolumeClaim" as StatefulSet?
   Also, are there any scenarios where protobuf is currently used?
   



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