fix: update option handling in ParseSetting and add test for overwrit…

…e behavior Signed-off-by: Xuhui zhang <[email protected]>
juicedata · Nov 1, 2024 · 899b4ef · 899b4ef
1 parent ac303d2
commit 899b4ef
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 17 deletions.
diff --git a/pkg/config/setting.go b/pkg/config/setting.go
@@ -244,7 +244,7 @@ func ParseSetting(secrets, volCtx map[string]string, options []string, usePod bo
 	if err := GenPodAttrWithCfg(&jfsSetting, volCtx); err != nil {
 		return nil, fmt.Errorf("GenPodAttrWithCfg error: %v", err)
 	}
-	if err := genAndValidOptions(&jfsSetting, options); err != nil {
+	if err := genAndValidOptions(&jfsSetting); err != nil {
 		return nil, fmt.Errorf("genAndValidOptions error: %v", err)
 	}
 	if err := GenCacheDirs(&jfsSetting, volCtx); err != nil {
@@ -373,9 +373,9 @@ func GenCacheDirs(jfsSetting *JfsSetting, volCtx map[string]string) error {
 	return nil
 }
 
-func genAndValidOptions(JfsSetting *JfsSetting, options []string) error {
+func genAndValidOptions(JfsSetting *JfsSetting) error {
 	mountOptions := []string{}
-	for _, option := range options {
+	for _, option := range JfsSetting.Options {
 		mountOption := strings.TrimSpace(option)
 		ops := strings.Split(mountOption, "=")
 		if len(ops) > 2 {

diff --git a/pkg/config/setting_test.go b/pkg/config/setting_test.go
@@ -65,6 +65,7 @@ func TestParseSecret(t *testing.T) {
 		secrets map[string]string
 		volCtx  map[string]string
 		options []string
+		patch   *MountPodPatch
 		usePod  bool
 	}
 	tests := []struct {
@@ -593,10 +594,38 @@ func TestParseSecret(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "overwrite options",
+			args: args{
+				secrets: map[string]string{"name": "test"},
+				options: []string{"buffer-size=10G"},
+				patch:   &MountPodPatch{MountOptions: []string{"buffer-size=100"}},
+			},
+			want: &JfsSetting{
+				Name:      "test",
+				Source:    "test",
+				Configs:   map[string]string{},
+				Envs:      map[string]string{},
+				Options:   []string{"buffer-size=100"},
+				CacheDirs: []string{"/var/jfsCache"},
+				Attr: &PodAttr{
+					Resources:            defaultResource,
+					JFSConfigPath:        JFSConfigPath,
+					Image:                "juicedata/mount:ee-nightly",
+					MountPointPath:       MountPointPath,
+					JFSMountPriorityName: JFSMountPriorityName,
+				},
+				CachePVCs: []CachePVC{},
+			},
+			wantErr: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			GlobalConfig.Reset()
+			defer GlobalConfig.Reset()
+			if tt.args.patch != nil {
+				GlobalConfig.MountPodPatch = []MountPodPatch{*tt.args.patch}
+			}
 			got, err := ParseSetting(tt.args.secrets, tt.args.volCtx, tt.args.options, tt.args.usePod, nil, nil)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("ParseSecret() error = %v, wantErr %v", err, tt.wantErr)
@@ -753,7 +782,6 @@ func Test_genCacheDirs(t *testing.T) {
 func Test_genAndValidOptions(t *testing.T) {
 	type args struct {
 		JfsSetting *JfsSetting
-		options    []string
 	}
 	tests := []struct {
 		name    string
@@ -764,35 +792,39 @@ func Test_genAndValidOptions(t *testing.T) {
 		{
 			name: "test-normal",
 			args: args{
-				JfsSetting: &JfsSetting{},
-				options:    []string{"cache-dir=xxx"},
+				JfsSetting: &JfsSetting{
+					Options: []string{"cache-dir=xxx"},
+				},
 			},
 			want:    []string{"cache-dir=xxx"},
 			wantErr: false,
 		},
 		{
 			name: "test-space1",
 			args: args{
-				JfsSetting: &JfsSetting{},
-				options:    []string{" cache-dir=xxx "},
+				JfsSetting: &JfsSetting{
+					Options: []string{" cache-dir=xxx "},
+				},
 			},
 			want:    []string{"cache-dir=xxx"},
 			wantErr: false,
 		},
 		{
 			name: "test-space2",
 			args: args{
-				JfsSetting: &JfsSetting{},
-				options:    []string{" cache-dir = xxx "},
+				JfsSetting: &JfsSetting{
+					Options: []string{" cache-dir = xxx "},
+				},
 			},
 			want:    []string{"cache-dir=xxx"},
 			wantErr: false,
 		},
 		{
 			name: "test-error",
 			args: args{
-				JfsSetting: &JfsSetting{},
-				options:    []string{"cache-dir=xxx cache-size=1024"},
+				JfsSetting: &JfsSetting{
+					Options: []string{"cache-dir=xxx cache-size=1024"},
+				},
 			},
 			want:    nil,
 			wantErr: true,
@@ -808,8 +840,8 @@ func Test_genAndValidOptions(t *testing.T) {
 							},
 						},
 					},
+					Options: []string{"buffer-size=1024"},
 				},
-				options: []string{"buffer-size=1024"},
 			},
 			want:    nil,
 			wantErr: true,
@@ -825,17 +857,17 @@ func Test_genAndValidOptions(t *testing.T) {
 							},
 						},
 					},
+					Options: []string{"buffer-size=1024M"},
 				},
-				options: []string{"buffer-size=1024M"},
 			},
 			want:    nil,
 			wantErr: true,
 		},
 		{
 			name: "test-buffersize-with-unit",
 			args: args{
-				options: []string{"buffer-size=10M"},
 				JfsSetting: &JfsSetting{
+					Options: []string{"buffer-size=10M"},
 					Attr: &PodAttr{
 						Resources: corev1.ResourceRequirements{
 							Limits: map[corev1.ResourceName]resource.Quantity{
@@ -851,11 +883,14 @@ func Test_genAndValidOptions(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			err := genAndValidOptions(tt.args.JfsSetting, tt.args.options)
+			err := genAndValidOptions(tt.args.JfsSetting)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("validOptions() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
+			if err != nil {
+				return
+			}
 			if !reflect.DeepEqual(tt.args.JfsSetting.Options, tt.want) {
 				t.Errorf("validOptions() got = %v, want %v", tt.args.JfsSetting.Options, tt.want)
 			}