This is an automated email from the ASF dual-hosted git repository.
xianjin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git
The following commit(s) were added to refs/heads/master by this push:
new be1069764 [#1290] improvement(operator): Avoid accidentally deleting
data of other services when misconfiguring the mounting directory (#1291)
be1069764 is described below
commit be106976474cc02a614ca842421670debf50f7aa
Author: QI Jiale <[email protected]>
AuthorDate: Sat Nov 18 14:41:56 2023 +0800
[#1290] improvement(operator): Avoid accidentally deleting data of other
services when misconfiguring the mounting directory (#1291)
### What changes were proposed in this pull request?
Only delete `rssdata` directory.
### Why are the changes needed?
Fix: #1290
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Tested in our cluster.
1. Mount `/data/hdfs1/rssdata1` dir for shuffle server，the server will
create `/data/hdfs1/rssdata1/rssdata/`
2. Create files manually at host machine
```
touch /data/hdfs1/rssdata1/a.txt
touch /data/hdfs1/rssdata1/rssdata/b.txt
```
3. Update crd to terminate this shuffle server.
4. Without this pr, both `a.txt`, `b.txt` and `rssdata` dir are deleted
5. With this pr, only `b.txt` is deleted
Co-authored-by: 齐家乐(26731624) <[email protected]>
---
.../operator/pkg/controller/constants/constants.go | 3 ++
.../controller/sync/shuffleserver/shuffleserver.go | 2 +-
.../operator/pkg/controller/util/util.go | 3 +-
.../operator/pkg/controller/util/util_test.go | 51 ++++++++++++++++++++++
4 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/deploy/kubernetes/operator/pkg/controller/constants/constants.go
b/deploy/kubernetes/operator/pkg/controller/constants/constants.go
index 8bda1ca6b..41c0f34c0 100644
--- a/deploy/kubernetes/operator/pkg/controller/constants/constants.go
+++ b/deploy/kubernetes/operator/pkg/controller/constants/constants.go
@@ -58,6 +58,9 @@ const (
// ConfigurationVolumeName is the name of configMap volume records
configuration of coordinators or shuffle servers.
ConfigurationVolumeName = "configuration"
+
+ //RssDataDir is the directory name for RSS data as the local storage.
+ RssDataDir = "rssdata"
)
// PropertyKey defines property key in configuration of coordinators or
shuffle servers.
diff --git
a/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver.go
b/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver.go
index 7b4baad69..6404456ba 100644
---
a/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver.go
+++
b/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver.go
@@ -236,7 +236,7 @@ func generateStorageBasePath(rss
*unifflev1alpha1.RemoteShuffleService) string {
if k == rss.Spec.ShuffleServer.LogHostPath {
continue
}
- paths = append(paths, strings.TrimSuffix(v, "/")+"/rssdata")
+ paths = append(paths, strings.TrimSuffix(v,
"/")+"/"+controllerconstants.RssDataDir)
}
sort.Strings(paths)
return strings.Join(paths, ",")
diff --git a/deploy/kubernetes/operator/pkg/controller/util/util.go
b/deploy/kubernetes/operator/pkg/controller/util/util.go
index 99cb0aff8..5a1375f43 100644
--- a/deploy/kubernetes/operator/pkg/controller/util/util.go
+++ b/deploy/kubernetes/operator/pkg/controller/util/util.go
@@ -86,7 +86,8 @@ func addVolumeMountsOfMainContainer(mainContainer
*corev1.Container,
mainContainer.VolumeMounts = append(mainContainer.VolumeMounts,
GenerateHostPathVolumeMounts(hostPathMounts)...)
for _, mountPath := range hostPathMounts {
- clearPathCMDs = append(clearPathCMDs, fmt.Sprintf("rm -rf
%v/*", strings.TrimSuffix(mountPath, "/")))
+ clearPathCMDs = append(clearPathCMDs, fmt.Sprintf("rm -rf
%v/%v/*",
+ strings.TrimSuffix(mountPath, "/"),
controllerconstants.RssDataDir))
}
if len(clearPathCMDs) > 0 {
mainContainer.Lifecycle = &corev1.Lifecycle{
diff --git a/deploy/kubernetes/operator/pkg/controller/util/util_test.go
b/deploy/kubernetes/operator/pkg/controller/util/util_test.go
index 864d8ee2d..d7e6d7a14 100644
--- a/deploy/kubernetes/operator/pkg/controller/util/util_test.go
+++ b/deploy/kubernetes/operator/pkg/controller/util/util_test.go
@@ -19,6 +19,7 @@ package util
import (
"sort"
+ "strings"
"testing"
"github.com/stretchr/testify/assert"
@@ -179,6 +180,56 @@ func TestAddOwnerReference(t *testing.T) {
}
}
+func TestAddVolumeMountsOfMainContainer(t *testing.T) {
+ for _, tt := range []struct {
+ name string
+ mainContainer *corev1.Container
+ hostPathMounts map[string]string
+ volumeMounts []corev1.VolumeMount
+ expectedCommand string
+ }{
+ {
+ name: "check add volume mount 1",
+ mainContainer: &corev1.Container{},
+ hostPathMounts: map[string]string{
+ "/rssdata1/data1": "/data1",
+ "/rssdata2/data2": "/data2",
+ "/rssdata3/data2": "/data3",
+ },
+ volumeMounts: []corev1.VolumeMount{},
+ expectedCommand: "rm -rf /data1/rssdata/*;rm -rf
/data2/rssdata/*;rm -rf /data3/rssdata/*",
+ },
+ {
+ name: "check add volume mount 2",
+ mainContainer: &corev1.Container{},
+ hostPathMounts: map[string]string{
+ "/rssdata4/data4": "/data4",
+ "/rssdata5/data5": "/data5",
+ "/rssdata6/data6": "/data6",
+ },
+ volumeMounts: []corev1.VolumeMount{
+ {
+ Name: "/rsslog",
+ MountPath: "/rsslog",
+ },
+ },
+ expectedCommand: "rm -rf /data4/rssdata/*;rm -rf
/data5/rssdata/*;rm -rf /data6/rssdata/*",
+ },
+ } {
+ t.Run(tt.name, func(t *testing.T) {
+ assertion := assert.New(t)
+ expectedVolumeMountCount := len(tt.hostPathMounts) +
len(tt.volumeMounts)
+ assertion.Equal(0, len(tt.mainContainer.VolumeMounts))
+ addVolumeMountsOfMainContainer(tt.mainContainer,
tt.hostPathMounts, tt.volumeMounts)
+ assertion.Equal(expectedVolumeMountCount,
len(tt.mainContainer.VolumeMounts))
+ strSlice :=
strings.Split(tt.mainContainer.Lifecycle.PreStop.Exec.Command[2], ";")
+ sort.Strings(strSlice)
+ sortedString := strings.Join(strSlice, ";")
+ assertion.Equal(tt.expectedCommand, sortedString)
+ })
+ }
+}
+
func buildRssWithUID() *uniffleapi.RemoteShuffleService {
rss := utils.BuildRSSWithDefaultValue()
rss.UID = "uid-test"
