This is an automated email from the ASF dual-hosted git repository. roryqi pushed a commit to branch branch-0.7 in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git
commit 72f718c40b2c84649da8e9b6cc70e5765ecd41aa Author: advancedxy <[email protected]> AuthorDate: Tue Feb 21 16:01:59 2023 +0800 [#632]fix: respect volumes in rss spec (#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. --- .../pkg/controller/sync/coordinator/coordinator.go | 22 ++++++------- .../sync/coordinator/coordinator_test.go | 37 ++++++++++++++++++++++ .../controller/sync/shuffleserver/shuffleserver.go | 22 ++++++------- .../sync/shuffleserver/shuffleserver_test.go | 37 ++++++++++++++++++++++ 4 files changed, 96 insertions(+), 22 deletions(-) diff --git a/deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go b/deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go index 0fccc65f..09fbee3e 100644 --- a/deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go +++ b/deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go @@ -184,21 +184,21 @@ func GenerateDeploy(rss *unifflev1alpha1.RemoteShuffleService, index int) *appsv Key: "node-role.kubernetes.io/master", }, }, - Volumes: []corev1.Volume{ - { - Name: controllerconstants.ConfigurationVolumeName, - VolumeSource: corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: rss.Spec.ConfigMapName, - }, - DefaultMode: pointer.Int32(0777), - }, + Volumes: rss.Spec.Coordinator.Volumes, + NodeSelector: rss.Spec.Coordinator.NodeSelector, + } + configurationVolume := corev1.Volume{ + Name: controllerconstants.ConfigurationVolumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: rss.Spec.ConfigMapName, }, + DefaultMode: pointer.Int32(0777), }, }, - NodeSelector: rss.Spec.Coordinator.NodeSelector, } + podSpec.Volumes = append(podSpec.Volumes, configurationVolume) if podSpec.HostNetwork { podSpec.DNSPolicy = corev1.DNSClusterFirstWithHostNet } diff --git a/deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator_test.go b/deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator_test.go index 361839b9..3da86e26 100644 --- a/deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator_test.go +++ b/deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator_test.go @@ -68,6 +68,19 @@ var ( Value: "127.0.0.1", }, } + testVolumeName = "test-volume" + testVolumes = []corev1.Volume{ + { + Name: testVolumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "test-config", + }, + }, + }, + }, + } ) func buildRssWithLabels() *uniffleapi.RemoteShuffleService { @@ -88,6 +101,12 @@ func buildRssWithCustomENVs() *uniffleapi.RemoteShuffleService { return rss } +func withCustomVolumes(volumes []corev1.Volume) *uniffleapi.RemoteShuffleService { + rss := utils.BuildRSSWithDefaultValue() + rss.Spec.Coordinator.Volumes = volumes + return rss +} + func buildRssWithCustomRPCPort() *uniffleapi.RemoteShuffleService { rss := utils.BuildRSSWithDefaultValue() rss.Spec.Coordinator.RPCPort = pointer.Int32(testRPCPort) @@ -308,6 +327,24 @@ func TestGenerateDeploy(t *testing.T) { return }, }, + { + name: "set custom volumes", + rss: withCustomVolumes(testVolumes), + IsValidDeploy: func(deploy *appsv1.Deployment, rss *uniffleapi.RemoteShuffleService) (bool, error) { + for _, volume := range deploy.Spec.Template.Spec.Volumes { + if volume.Name == testVolumeName { + expectedVolume := testVolumes[0] + equal := reflect.DeepEqual(expectedVolume, volume) + if equal { + return true, nil + } + volumeJSON, _ := json.Marshal(expectedVolume) + return false, fmt.Errorf("generated deploy doesn't contain expected volumn: %s", volumeJSON) + } + } + return false, fmt.Errorf("generated deploy should include volume: %s", testVolumeName) + }, + }, } { t.Run(tt.name, func(tc *testing.T) { deploy := GenerateDeploy(tt.rss, 0) diff --git a/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver.go b/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver.go index cb77d0af..ea531d48 100644 --- a/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver.go +++ b/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver.go @@ -91,21 +91,21 @@ func GenerateSts(rss *unifflev1alpha1.RemoteShuffleService) *appsv1.StatefulSet Key: "node-role.kubernetes.io/master", }, }, - Volumes: []corev1.Volume{ - { - Name: controllerconstants.ConfigurationVolumeName, - VolumeSource: corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: rss.Spec.ConfigMapName, - }, - DefaultMode: pointer.Int32(0777), - }, + Volumes: rss.Spec.ShuffleServer.Volumes, + NodeSelector: rss.Spec.ShuffleServer.NodeSelector, + } + configurationVolume := corev1.Volume{ + Name: controllerconstants.ConfigurationVolumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: rss.Spec.ConfigMapName, }, + DefaultMode: pointer.Int32(0777), }, }, - NodeSelector: rss.Spec.ShuffleServer.NodeSelector, } + podSpec.Volumes = append(podSpec.Volumes, configurationVolume) if podSpec.HostNetwork { podSpec.DNSPolicy = corev1.DNSClusterFirstWithHostNet } diff --git a/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver_test.go b/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver_test.go index 80472b12..38c64b9e 100644 --- a/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver_test.go +++ b/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver_test.go @@ -68,6 +68,19 @@ var ( Value: "1G", }, } + testVolumeName = "test-volume" + testVolumes = []corev1.Volume{ + { + Name: testVolumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "test-config", + }, + }, + }, + }, + } ) func buildRssWithLabels() *uniffleapi.RemoteShuffleService { @@ -93,6 +106,12 @@ func buildRssWithCustomENVs() *uniffleapi.RemoteShuffleService { return rss } +func withCustomVolumes(volumes []corev1.Volume) *uniffleapi.RemoteShuffleService { + rss := utils.BuildRSSWithDefaultValue() + rss.Spec.ShuffleServer.Volumes = volumes + return rss +} + func buildRssWithCustomRPCPort() *uniffleapi.RemoteShuffleService { rss := utils.BuildRSSWithDefaultValue() rss.Spec.ShuffleServer.RPCPort = pointer.Int32(testRPCPort) @@ -315,6 +334,24 @@ 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 { + expectedVolume := testVolumes[0] + equal := reflect.DeepEqual(expectedVolume, volume) + if equal { + return true, nil + } + volumeJSON, _ := json.Marshal(expectedVolume) + return false, fmt.Errorf("generated sts doesn't contain expected volumn: %s", volumeJSON) + } + } + return false, fmt.Errorf("generated sts should include volume: %s", testVolumeName) + }, + }, } { t.Run(tt.name, func(tc *testing.T) { sts := GenerateSts(tt.rss)
